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 604008 - GBR, PAT: sanitize input data
GBR, PAT: sanitize input data
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 17:39 UTC by Nils Philippsen
Modified: 2009-12-09 16:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed fix: sanitize input data and use correct types in the GBR plugin (1.35 KB, patch)
2009-12-07 17:39 UTC, Nils Philippsen
none Details | Review
Proposed fix: sanitize input data and use correct types in the PAT plugin (1.06 KB, patch)
2009-12-07 17:40 UTC, Nils Philippsen
none Details | Review
Proposed fix: sanitize input data and use correct types in the PAT plugin (1.61 KB, patch)
2009-12-07 17:48 UTC, Nils Philippsen
none Details | Review
Proposed fix: sanitize input data and use correct types in the GBR plugin (amended) (1.27 KB, patch)
2009-12-08 15:49 UTC, Nils Philippsen
committed Details | Review
Proposed fix: sanitize input data and use correct types in the PAT plugin (amended) (1.78 KB, patch)
2009-12-08 16:47 UTC, Nils Philippsen
committed Details | Review

Description Nils Philippsen 2009-12-07 17:39:41 UTC
Created attachment 149278 [details] [review]
Proposed fix: sanitize input data and use correct types in the GBR plugin

Information contained in GBR and PAT files are used in their respective plugins without being sanitized frist, potentially allowing overflows. While g_malloc()/G_free() are immune against the value 0, calculating the amount of memory to allocate may again overflow the target type. Additionally, the plugins use the wrong types for such calculations (gssize instead of gsize in GBR, automatic type selection in PAT).
Comment 1 Nils Philippsen 2009-12-07 17:40:23 UTC
Created attachment 149279 [details] [review]
Proposed fix: sanitize input data and use correct types in the PAT plugin
Comment 2 Nils Philippsen 2009-12-07 17:48:38 UTC
Created attachment 149281 [details] [review]
Proposed fix: sanitize input data and use correct types in the PAT plugin

Revised the above patch.
Comment 3 Simon Budig 2009-12-07 22:06:18 UTC
If adding sanitization to a plugin please do it the full way, i.e.

- check for >0 *and* for <= GIMP_MAX_IMAGE_SIZE.
- Do it for width *and* height, regardless of possibly misfiring calculations
- then do the calculation checks.

And then I don't get the reasoning in your revised patch. What is the reasoning behind the casts?
Comment 4 Nils Philippsen 2009-12-08 15:49:16 UTC
Created attachment 149342 [details] [review]
Proposed fix: sanitize input data and use correct types in the GBR plugin (amended)

Add checks for GIMP_MAX_IMAGE_SIZE as well as valid values of bh.bytes.
Comment 5 Nils Philippsen 2009-12-08 16:47:42 UTC
Created attachment 149352 [details] [review]
Proposed fix: sanitize input data and use correct types in the PAT plugin (amended)

Checks both width and height and against GIMP_MAX_IMAGE_SIZE. Doesn't unnecessarily cast allocation calculations (because GIMP_MAX_IMAGE_SIZE << GIMP_MAXUINT32), but adds explanatory comments.
Comment 6 Simon Budig 2009-12-09 15:12:09 UTC
Review of attachment 149352 [details] [review]:

Looks good, please commit.
Comment 7 Simon Budig 2009-12-09 15:13:32 UTC
Review of attachment 149342 [details] [review]:

looks good, please commit.
Comment 8 Nils Philippsen 2009-12-09 16:04:19 UTC
Review of attachment 149342 [details] [review]:

committed:
869dcd7 GBR: sanitize input data
b053021 GBR: more input data sanitation
Comment 9 Nils Philippsen 2009-12-09 16:05:25 UTC
Review of attachment 149352 [details] [review]:

committed:
5aa82f3 PAT: sanitize input data
Comment 10 Nils Philippsen 2009-12-09 16:05:47 UTC
Review of attachment 149352 [details] [review]:

committed:
5aa82f3 PAT: sanitize input data