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 603995 - PCX plugin doesn't sanitize input to avoid allocation overflows.
PCX plugin doesn't sanitize input to avoid allocation overflows.
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: Plugins
git master
Other Linux
: Normal major
: ---
Assigned To: GIMP Bugs
GIMP Bugs
Depends on:
Blocks:
 
 
Reported: 2009-12-07 16:16 UTC by Nils Philippsen
Modified: 2009-12-09 15:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch: ensure that too high input data doesn't overflow allocation calculations (800 bytes, patch)
2009-12-07 16:16 UTC, Nils Philippsen
accepted-commit_now Details | Review
Proposed fix: sanitize input data (amended) (815 bytes, patch)
2009-12-08 14:00 UTC, Nils Philippsen
committed Details | Review

Description Nils Philippsen 2009-12-07 16:16:48 UTC
Created attachment 149267 [details] [review]
Proposed patch: ensure that too high input data doesn't overflow allocation calculations

Depending on the image type (b/w, indexed, grayscale, RGB), the PCX plugin tries to allocate memory depending on width and height stored in the file header. It doesn't check whether calculating the amount of memory to allocate would overflow the target type (gsize).

At least in the case of RGB, the amount of memory needed can be overflowed as it is calculated as "width * height * 3". Both width and height are unsigned 16bit values kept in signed 32bit variables -- if both are G_MAXUINT16, the result of this calculation would overflow 32bit types (like gsize on x86 32bit machines).
Comment 1 Simon Budig 2009-12-07 21:11:59 UTC
Review of attachment 149267 [details] [review]:

There is a typo in the message (should be "too") and if a width can be too high   :)
(I think I'd prefer "big", not sure.

Otherwise the patch looks fine, please commit.
Comment 2 Sven Neumann 2009-12-07 21:33:49 UTC
The best would be to try not to introduce a new string, but use one that is already in the po-plugins message domain. But that can be done as an extra step and it is only really needed in the gimp-2-6 branch.
Comment 3 Nils Philippsen 2009-12-08 13:50:17 UTC
Hmm, I can't find a suitable message in po-plug-ins... I would change the text to "Image dimensions too large: width %d x height %d" in master, but what should I do in gimp-2-6?
Comment 4 Nils Philippsen 2009-12-08 14:00:25 UTC
Created attachment 149328 [details] [review]
Proposed fix: sanitize input data (amended)
Comment 5 Sven Neumann 2009-12-08 19:37:45 UTC
If there is no suitable string, then we need to introduce a new one.
Comment 6 Nils Philippsen 2009-12-09 09:40:39 UTC
I think in the case of gimp-2-6, it is acceptable if the error message is not translated right away (it should only be ever visible in unusual circumstances). If you agree, I would commit the fix.
Comment 7 Simon Budig 2009-12-09 14:32:56 UTC
Review of attachment 149328 [details] [review]:

Looks good. Please commit.
Comment 8 Nils Philippsen 2009-12-09 15:39:09 UTC
Review of attachment 149328 [details] [review]:

committed as ed7f48be05d2