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 790783 - (CVE-2017-17788) buffer overread in XCF parser if version field has no null terminator
(CVE-2017-17788)
buffer overread in XCF parser if version field has no null terminator
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: General
2.9.6
Other All
: Normal normal
: 2.10
Assigned To: GIMP Bugs
GIMP Bugs
Depends on:
Blocks:
 
 
Reported: 2017-11-24 11:39 UTC by Hanno Böck
Modified: 2017-12-26 14:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
sample file triggering overread. (724 bytes, image/x-xcf)
2017-11-24 11:39 UTC, Hanno Böck
  Details
stack trace / asan (4.67 KB, text/plain)
2017-11-24 11:39 UTC, Hanno Böck
  Details
patch / fix (432 bytes, patch)
2017-11-24 11:39 UTC, Hanno Böck
committed Details | Review

Description Hanno Böck 2017-11-24 11:39:18 UTC
Created attachment 364313 [details]
sample file triggering overread.

I'll attach a file that will cause a stack overread in the XCF file import. This was discovered by fuzzing with american fuzzy lop and address sanitizer. I'll also attach the stack trace from asan. The overread can be detected by compiling gimp with address sanitizer.

The bug is in xcf.c when reading the file version. According to the inofficial XCF spec [1] the version is a string starting at offset 9 with a null terminator at offset 13.

The code in xcf.c assumes that this null terminator is there and passes the version string to atoi. So if you craft a file where it's missing then atoi will overread. This can be fixed by checking that the null terminator is really set to 0 and returning an error if it's not. Patch also attached.

[1] http://henning.makholm.net/xcftools/xcfspec-saved
Comment 1 Hanno Böck 2017-11-24 11:39:36 UTC
Created attachment 364314 [details]
stack trace / asan
Comment 2 Hanno Böck 2017-11-24 11:39:53 UTC
Created attachment 364315 [details] [review]
patch / fix
Comment 3 Michael Natterer 2017-11-26 23:50:43 UTC
The added condition in the patch was always TRUE, I fixed this and pushed:

commit 702c4227e8b6169f781e4bb5ae4b5733f51ab126
Author: Hanno Boeck <hanno@hboeck.de>
Date:   Mon Nov 27 00:37:29 2017 +0100

    790783 - buffer overread in XCF parser if version field...
    
    ...has no null terminator
    
    Check for the presence of '\0' before using atoi() on the version
    string. Patch slightly modified (mitch).

 app/xcf/xcf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Comment 4 Emilio Pozuelo Monfort 2017-12-23 15:41:30 UTC
Maybe this should be backported to gimp-2-8 ? 2.8.x seems affected, though the function there is called xcf_load_invoker().
Comment 5 Salvatore Bonaccorso 2017-12-26 14:52:07 UTC
FTR: The xcf_load_stream was factored out in 63bcc698270bea1c4daa61c94da2c6c23dab87c5 .