Commit e82aaa4b authored by Shmuel H's avatar Shmuel H Committed by Michael Natterer

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 6d804bf9)
parent 25dd2c33
...@@ -904,6 +904,18 @@ xcf_load_layer_props (XcfInfo *info, ...@@ -904,6 +904,18 @@ xcf_load_layer_props (XcfInfo *info,
case PROP_GROUP_ITEM: case PROP_GROUP_ITEM:
{ {
GimpLayer *group; GimpLayer *group;
gboolean is_active_layer;
/* We're going to delete *layer, Don't leave its pointers
* in @info. After that, we'll restore them back with the
* new pointer. See bug #767873.
*/
is_active_layer = (*layer == info->active_layer);
if (is_active_layer)
info->active_layer = NULL;
if (*layer == info->floating_sel)
info->floating_sel = NULL;
group = gimp_group_layer_new (image); group = gimp_group_layer_new (image);
...@@ -916,6 +928,13 @@ xcf_load_layer_props (XcfInfo *info, ...@@ -916,6 +928,13 @@ xcf_load_layer_props (XcfInfo *info,
g_object_ref_sink (*layer); g_object_ref_sink (*layer);
g_object_unref (*layer); g_object_unref (*layer);
*layer = group; *layer = group;
if (is_active_layer)
info->active_layer = *layer;
/* Don't restore info->floating_sel because group layers
* can't be floating selections
*/
} }
break; break;
...@@ -986,6 +1005,12 @@ xcf_load_channel_props (XcfInfo *info, ...@@ -986,6 +1005,12 @@ xcf_load_channel_props (XcfInfo *info,
{ {
GimpChannel *mask; GimpChannel *mask;
/* We're going to delete *channel, Don't leave its pointer
* in @info. See bug #767873.
*/
if (*channel == info->active_channel)
info->active_channel = NULL;
mask = mask =
gimp_selection_new (image, gimp_selection_new (image,
gimp_item_get_width (GIMP_ITEM (*channel)), gimp_item_get_width (GIMP_ITEM (*channel)),
...@@ -1000,6 +1025,10 @@ xcf_load_channel_props (XcfInfo *info, ...@@ -1000,6 +1025,10 @@ xcf_load_channel_props (XcfInfo *info,
*channel = mask; *channel = mask;
(*channel)->boundary_known = FALSE; (*channel)->boundary_known = FALSE;
(*channel)->bounds_known = FALSE; (*channel)->bounds_known = FALSE;
/* Don't restore info->active_channel because the
* selection can't be the active channel
*/
} }
break; break;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment