After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 778943 - Various miscellaneous bounds checking fixes
Various miscellaneous bounds checking fixes
Status: RESOLVED OBSOLETE
Product: gdk-pixbuf
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gdk-pixbuf-maint
gdk-pixbuf-maint
Depends on:
Blocks:
 
 
Reported: 2017-02-20 09:29 UTC by Philip Withnall
Modified: 2018-05-22 13:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
io-ico: Add an assertion to clarify buffer management (1.13 KB, patch)
2017-02-20 09:29 UTC, Philip Withnall
committed Details | Review
timescale: Check bounds of command line dimension arguments (1.33 KB, patch)
2017-02-20 09:29 UTC, Philip Withnall
needs-work Details | Review
tests: Add some assertions to check for zero-dimensioned images (3.67 KB, patch)
2017-02-20 09:29 UTC, Philip Withnall
committed Details | Review

Description Philip Withnall 2017-02-20 09:29:36 UTC
None of these fix any real bugs — they’re all to pacify the static analyser, or are in tests or utilities. Still, they can’t hurt.
Comment 1 Philip Withnall 2017-02-20 09:29:40 UTC
Created attachment 346242 [details] [review]
io-ico: Add an assertion to clarify buffer management

The code is correct (the line buffer is set when DecodeHeader() returns
successfully with a non-zero-length line), but that’s not at all obvious
from the code, and is tripping Coverity up. Add an assertion to make it
clearer.

Coverity ID: 1400057
Comment 2 Philip Withnall 2017-02-20 09:29:46 UTC
Created attachment 346243 [details] [review]
timescale: Check bounds of command line dimension arguments

This is only a test utility, but in order to shut Coverity up we might
as well add appropriate bounds checking to its command line width and
height arguments.

Coverity IDs: 1388527, 1388541, 1388542
Comment 3 Philip Withnall 2017-02-20 09:29:51 UTC
Created attachment 346244 [details] [review]
tests: Add some assertions to check for zero-dimensioned images

This could happen if something in the test fails, so this allows early
diagnosis of problems. It also hints to Coverity that the loops which
follow can’t run (almost) infinitely due to the loop bounds being
inverted.

Coverity IDs: 1391987, 1399712
Comment 4 Philip Withnall 2017-05-25 13:12:28 UTC
Ping?
Comment 5 Bastien Nocera 2017-07-28 16:02:30 UTC
Review of attachment 346242 [details] [review]:

Looks good.
Comment 6 Bastien Nocera 2017-07-28 16:03:35 UTC
Review of attachment 346243 [details] [review]:

This should probably be rewritten to use the calculate_rowstride instead, right?
Comment 7 Bastien Nocera 2017-07-28 16:04:14 UTC
Review of attachment 346244 [details] [review]:

Sure.
Comment 8 Philip Withnall 2017-08-03 15:23:59 UTC
Attachment 346242 [details] pushed as 48accc7 - io-ico: Add an assertion to clarify buffer management
Attachment 346244 [details] pushed as 861a6db - tests: Add some assertions to check for zero-dimensioned images
Comment 9 Philip Withnall 2017-08-09 09:59:01 UTC
(In reply to Bastien Nocera from comment #6)
> Review of attachment 346243 [details] [review] [review]:
> 
> This should probably be rewritten to use the calculate_rowstride instead,
> right?

Looking at it, the calculations here are slightly different to the ones in calculate_rowstride() wrt has_alpha vs (src_index == 0). I’m not sure how to reconcile them, and would rather just push this patch than try and work out what’s going on. This patch is correct wrt the existing code in timescale.c, and its checks are happening sufficiently early on that we’re not allocating GBs of memory for incorrect parameters before checking them.
Comment 10 Philip Withnall 2017-08-15 09:05:54 UTC
Ping?
Comment 11 GNOME Infrastructure Team 2018-05-22 13:22:44 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gdk-pixbuf/issues/63.