Use after free in `pango_attr_list_change`
There is a frequently-occurring use-after-free bug in pango_attr_list_change
, which in rare circumstances can lead to a null-pointer dereference and subsequent crash.
Use-after-free
Here is the source code for pango_attr_list_change
, trimmed and with three important lines annotated:
void
pango_attr_list_change (PangoAttrList *list,
PangoAttribute *attr)
{
// ...
if (!inserted)
/* we didn't insert attr yet */
pango_attr_list_insert (list, attr); // (A)
/* We now have the range inserted into the list one way or the
* other. Fix up the remainder
*/
/* Attention: No i = 0 here. */
for (i = i + 1, p = list->attributes->len; i < p; i++)
{
PangoAttribute *tmp_attr = g_ptr_array_index (list->attributes, i);
if (tmp_attr->start_index > end_index)
break;
if (tmp_attr->klass->type != attr->klass->type) // (B)
continue;
if (tmp_attr->end_index <= attr->end_index ||
pango_attribute_equal (tmp_attr, attr))
{
/* We can merge the new attribute with this attribute. */
attr->end_index = MAX (end_index, tmp_attr->end_index);
pango_attribute_destroy (tmp_attr); // (C)
g_ptr_array_remove_index (list->attributes, i);
i--;
p--;
continue;
}
else
{
// ...
}
}
}
The following sequence of events can lead to a use-after-free:
-
Line (A) inserts
attr
intolist->attributes
. -
The loop then begins executing, with
tmp_attr
taking on successive value inlist->attributes
. -
Eventually the loop hits the point where
tmp_attr == attr
. At line (C),tmp_attr
is freed. -
At line (B),
attr
is read. (Use-after-free.)
Null-pointer dereference
On its own, the above bug isn't terribly serious, as the memory for attr
continues to hold the same data.
However, if another thread allocates and zeroes a block of memory, this can zero out attr
while the above function is still running. Consequently attr->klass
is null, and so attr->klass->type
crashes with a null-pointer dereference at line (B).
Real-world implications
In Inkscape, the use-after-free occurs hundreds of times per second while dragging text, but is otherwise harmless.
The null-pointer dereference is much harder to reproduce, but is responsible for Inkscape's bug 2944. (Currently the most severe bug in the bug tracker and the only release-blocker.)
Fix
Quitting the function after deleting attr
fixes the crash for me:
/* We can merge the new attribute with this attribute. */
attr->end_index = MAX (end_index, tmp_attr->end_index);
pango_attribute_destroy (tmp_attr);
g_ptr_array_remove_index (list->attributes, i); // (C)
+if (tmp_attr == attr) break;
i--;
p--;
continue;
That may or may not be the right thing to do, but it's a good enough start, and provides more evidence the above theories are correct.