diff --git a/cJSON_Utils.c b/cJSON_Utils.c index 4f3a9ed8d..df174e639 100644 --- a/cJSON_Utils.c +++ b/cJSON_Utils.c @@ -589,6 +589,16 @@ static cJSON *sort_list(cJSON *list, const cJSON_bool case_sensitive) second->prev = result_tail; } + /* Restore the list invariant: the head's prev points to the tail + * (add_item_to_array() relies on it for O(1) appends). The merge above + * only maintains prev of appended elements, so result->prev is stale. */ + current_item = result; + while (current_item->next != NULL) + { + current_item = current_item->next; + } + result->prev = current_item; + return result; } diff --git a/tests/misc_utils_tests.c b/tests/misc_utils_tests.c index 7d300bc8e..16afc0bf6 100644 --- a/tests/misc_utils_tests.c +++ b/tests/misc_utils_tests.c @@ -70,11 +70,48 @@ static void cjson_utils_functions_shouldnt_crash_with_null_pointers(void) cJSON_Delete(item); } +static void sort_object_should_restore_prev_tail_invariant(void) +{ + /* cJSON relies on child->prev pointing to the list tail (used by + * add_item_to_array() for O(1) appends). The mergesort in sort_list() + * must restore that invariant, otherwise a later delete+add performs a + * use-after-free write into the stale prev node (issue #1090). */ + cJSON *object = cJSON_Parse("{\"a\":1,\"b\":1,\"c\":1,\"e\":1,\"d\":1}"); + cJSON *tail = NULL; + TEST_ASSERT_NOT_NULL(object); + + cJSONUtils_SortObject(object); + + tail = object->child; + while ((tail != NULL) && (tail->next != NULL)) + { + tail = tail->next; + } + TEST_ASSERT_NOT_NULL(tail); + TEST_ASSERT_EQUAL_PTR(tail, object->child->prev); + + /* delete the interior node head->prev used to point at, then append */ + cJSON_DeleteItemFromObject(object, "d"); + cJSON_AddItemToObject(object, "z", cJSON_CreateNumber(1)); + + tail = object->child; + while ((tail != NULL) && (tail->next != NULL)) + { + tail = tail->next; + } + TEST_ASSERT_NOT_NULL(tail); + TEST_ASSERT_EQUAL_STRING("z", tail->string); + TEST_ASSERT_EQUAL_PTR(tail, object->child->prev); + + cJSON_Delete(object); +} + int main(void) { UNITY_BEGIN(); RUN_TEST(cjson_utils_functions_shouldnt_crash_with_null_pointers); + RUN_TEST(sort_object_should_restore_prev_tail_invariant); return UNITY_END(); }