UAF due to insufficient object cleanup when object creation fails

[1ndahous3]

There is a lack of additional cleanup of an object if it was not fully constructed in yr_object_function_create() and is being destroyed by yr_object_destroy() in the FAIL_ON_ERROR_WITH_CLEANUP() macro:

yara/libyara/object.c

Lines 217 to 224 in d7e8be3

	if (f == NULL) // Function doesn't exist yet
	{
	FAIL_ON_ERROR(
	yr_object_create(OBJECT_TYPE_FUNCTION, identifier, parent, &o));
	FAIL_ON_ERROR_WITH_CLEANUP(
	yr_object_create(return_type, "result", o, &return_obj),
	yr_object_destroy(o));

There are 3 objects: parent, o and return_obj, and when we successfully created the o object, some parent field became a reference to o:

yara/libyara/object.c

Lines 142 to 163 in d7e8be3

	switch (parent->type)
	{
	case OBJECT_TYPE_STRUCTURE:
	FAIL_ON_ERROR_WITH_CLEANUP(yr_object_structure_set_member(parent, obj), {
	yr_free((void*) obj->identifier);
	yr_free(obj);
	});
	break;
	case OBJECT_TYPE_ARRAY:
	object_as_array(parent)->prototype_item = obj;
	break;
	case OBJECT_TYPE_DICTIONARY:
	object_as_dictionary(parent)->prototype_item = obj;
	break;
	case OBJECT_TYPE_FUNCTION:
	object_as_function(parent)->return_obj = obj;
	break;
	}
	}

But when we fail to create a return_obj, we do yr_object_destroy(o) as a cleanup action, which only affects the data of the o object, not the parent object.

In this case, the object_as_structure(parent)->members pointer is no longer valid.

We get several functions that can perform the UAF of this pointer in the same way:

yara/libyara/object.c

Lines 323 to 333 in d7e8be3

	case OBJECT_TYPE_STRUCTURE:
	member = object_as_structure(object)->members;
	while (member != NULL)
	{
	next_member = member->next;
	yr_object_destroy(member->object);
	yr_free(member);
	member = next_member;
	}
	break;

yara/libyara/object.c

Lines 393 to 401 in d7e8be3

	member = object_as_structure(object)->members;
	while (member != NULL)
	{
	if (strcmp(member->object->identifier, field_name) == 0)
	return member->object;
	member = member->next;
	}

Detected by the Application Verifier (Windows) with the low resource simulation feature:

The fix could be to additionally check the object->parent field at the end of the object destruction in yr_object_destroy() and fix the fields according to the initialization of the parent yr_object_create():

void yr_object_destroy(YR_OBJECT* object)
{
  YR_STRUCTURE_MEMBER* member;
  YR_STRUCTURE_MEMBER* next_member;
  YR_ARRAY_ITEMS* array_items;
  YR_DICTIONARY_ITEMS* dict_items;

...
  // start of new code
  if (object->parent != NULL)
  {
    switch (object->parent->type)
    {
    case OBJECT_TYPE_STRUCTURE:
      object_as_structure(object->parent)->members = NULL;
      break;

    case OBJECT_TYPE_ARRAY:
      object_as_array(object->parent)->prototype_item = NULL;
      break;

    case OBJECT_TYPE_DICTIONARY:
      object_as_dictionary(object->parent)->prototype_item = NULL;
      break;

    case OBJECT_TYPE_FUNCTION:
      object_as_function(object->parent)->return_obj = NULL;
      break;
    }
  }
  // end of new code

  yr_free((void*) object->identifier);
  yr_free(object);
}