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 767873 - (CVE-2016-4994) Multiple Use-After-Free when parsing XCF channel and layer properties
(CVE-2016-4994)
Multiple Use-After-Free when parsing XCF channel and layer properties
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: General
2.9.2
Other All
: Normal normal
: 2.8
Assigned To: GIMP Bugs
GIMP Bugs
Depends on:
Blocks:
 
 
Reported: 2016-06-20 15:18 UTC by shmuelgimp
Modified: 2016-06-26 16:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (3.34 KB, patch)
2016-06-20 15:18 UTC, shmuelgimp
committed Details | Review
Gimp_UaF.xcf (11.16 KB, image/x-xcf)
2016-06-20 15:20 UTC, shmuelgimp
  Details

Description shmuelgimp 2016-06-20 15:18:18 UTC
Created attachment 330078 [details] [review]
Patch

The properties PROP_ACTIVE_LAYER, PROP_FLOATING_SELECTION, PROP_ACTIVE_CHANNEL saves the current object pointer the @info structure. Others like PROP_SELECTION (for channel) and PROP_GROUP_ITEM (for layer) will delete the current object and create a new object, leaving the pointers in @info invalid (dangling).

Therefore, if a property from the first type will come before the second, the result will be an UaF in the last lines of xcf_load_image (when it actually using the pointers from @info).

I wasn't able to exploit this bug because that g_type_instance->c_class gets cleared by the last g_object_unref and GIMP_IS_{LAYER,CHANNEL} detects that and return FALSE. It isn't possible to put a fake a GTypeInstanceClass and put it instead because that the GType is kind of random (based on the address of the GTypeInstanceClass which is the return value of g_malloc).

I have attached a patch that should fix that and a XCF file that will trigger this bug.

Example for running gimp_uaf.xcf on master:
$ gimp Gimp_UaF.xcf
This is a development version of GIMP.  Debug messages may appear here.
...
(gimp:18010): Gimp-Core-CRITICAL **: gimp_image_set_active_layer: assertion 'layer == NULL || GIMP_IS_LAYER (layer)' failed
Comment 1 shmuelgimp 2016-06-20 15:20:30 UTC
Created attachment 330079 [details]
Gimp_UaF.xcf
Comment 2 Michael Natterer 2016-06-20 20:18:32 UTC
I don't think this can ever happen unles one constructs such an XCF
artificially (as in now using GIMP's xcf saver). Is this such an XCF?
Comment 3 Michael Natterer 2016-06-20 20:21:41 UTC
...as in *not* using...
Comment 4 shmuelgimp 2016-06-20 20:30:18 UTC
(In reply to Michael Natterer from comment #2)
> I don't think this can ever happen unles one constructs such an XCF
> artificially (as in now using GIMP's xcf saver). Is this such an XCF?

Yes, it is. But I think that this is a security bug.

It is possible to heap-spray a malicious Layer (or Channel) and override its class pointer (and then it's a RCE when any virtual function get called). The only problem is with the GType that is random.
Comment 5 Michael Natterer 2016-06-20 20:49:01 UTC
Yes it is a security bug, just wanted to know how the file happened :)
Comment 6 shmuelgimp 2016-06-20 20:53:12 UTC
(In reply to Michael Natterer from comment #5)
> Yes it is a security bug, just wanted to know how the file happened :)

Ok. If I remember correctly, I just added PROP_ACTIVE_LAYER to an Group Layer.
By the way, it happened with both 2.9 (master) and 2.8.16.
Comment 7 shmuelgimp 2016-06-21 15:56:02 UTC
The CVE-ID for this bug is CVE-2016-4994.
Comment 8 Michael Natterer 2016-06-22 10:50:21 UTC
Pushed a modified version to master and gimp-2-8, the modifications
make it even safer:

commit e82aaa4b4ee0703c879e35ea9321fff6be3e9b6f
Author: Shmuel H <shmuelgimp@gmail.com>
Date:   Mon Jun 20 17:14:41 2016 +0300

    Bug 767873 - (CVE-2016-4994) Multiple Use-After-Free when parsing...
    
    ...XCF channel and layer properties
    
    The properties PROP_ACTIVE_LAYER, PROP_FLOATING_SELECTION,
    PROP_ACTIVE_CHANNEL saves the current object pointer the @info
    structure. Others like PROP_SELECTION (for channel) and
    PROP_GROUP_ITEM (for layer) will delete the current object and create
    a new object, leaving the pointers in @info invalid (dangling).
    
    Therefore, if a property from the first type will come before the
    second, the result will be an UaF in the last lines of xcf_load_image
    (when it actually using the pointers from @info).
    
    I wasn't able to exploit this bug because that
    g_object_instance->c_class gets cleared by the last g_object_unref and
    GIMP_IS_{LAYER,CHANNEL} detects that and return FALSE.
    
    (cherry picked from commit 6d804bf9ae77bc86a0a97f9b944a129844df9395)

 app/xcf/xcf-load.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)