Fix invalid memory read for table with many flag/pair ids

Author: SanderMertens

distr/flecs.c

@@ -36771,7 +36771,7 @@ void flecs_table_init(
      * we're adding the next set of records */
     if (first_role != -1 || first_pair != -1) {
         int32_t start = first_role;
-        if (first_pair != -1 && (start != -1 || first_pair < start)) {
+        if (first_pair != -1 && (start == -1 || first_pair < start)) {
             start = first_pair;
         }
 
@@ -36786,6 +36786,10 @@ void flecs_table_init(
         ecs_vec_set_min_size_t(a, records, ecs_table_record_t, record_count);
     }
 
+    /* Get records size now so we can check that array did not resize */
+    int32_t records_size = ecs_vec_size(records);
+    (void)records_size;
+
     /* Add records for ids with roles (used by cleanup logic) */
     if (first_role != -1) {
         for (dst_i = first_role; dst_i < dst_count; dst_i ++) {
@@ -36902,6 +36906,11 @@ void flecs_table_init(
             /* If this is a target wildcard record it has already been 
              * registered, but the record is now at a different location in
              * memory. Patch up the linked list with the new address */
+
+            /* Ensure that record array hasn't been reallocated */
+            ecs_assert(records_size == ecs_vec_size(records), 
+                ECS_INTERNAL_ERROR, NULL);
+
             ecs_table_cache_replace(&idr->cache, table, &tr->hdr);
         } else {
             /* Other records are not registered yet */

src/storage/table.c

@@ -447,7 +447,7 @@ void flecs_table_init(
      * we're adding the next set of records */
     if (first_role != -1 || first_pair != -1) {
         int32_t start = first_role;
-        if (first_pair != -1 && (start != -1 || first_pair < start)) {
+        if (first_pair != -1 && (start == -1 || first_pair < start)) {
             start = first_pair;
         }
 
@@ -462,6 +462,10 @@ void flecs_table_init(
         ecs_vec_set_min_size_t(a, records, ecs_table_record_t, record_count);
     }
 
+    /* Get records size now so we can check that array did not resize */
+    int32_t records_size = ecs_vec_size(records);
+    (void)records_size;
+
     /* Add records for ids with roles (used by cleanup logic) */
     if (first_role != -1) {
         for (dst_i = first_role; dst_i < dst_count; dst_i ++) {
@@ -578,6 +582,11 @@ void flecs_table_init(
             /* If this is a target wildcard record it has already been 
              * registered, but the record is now at a different location in
              * memory. Patch up the linked list with the new address */
+
+            /* Ensure that record array hasn't been reallocated */
+            ecs_assert(records_size == ecs_vec_size(records), 
+                ECS_INTERNAL_ERROR, NULL);
+
             ecs_table_cache_replace(&idr->cache, table, &tr->hdr);
         } else {
             /* Other records are not registered yet */

test/core/project.json

@@ -2410,7 +2410,8 @@
                 "clear_table_check_size",
                 "clear_table_twice_check_size",
                 "clear_table_on_remove_hooks",
-                "clear_table_on_remove_observer"
+                "clear_table_on_remove_observer",
+                "65_records_w_tgt"
             ]
         }, {
             "id": "Poly",

test/core/src/Table.c

@@ -765,7 +765,7 @@ void Observer(ecs_iter_t *it) {
 }
 
 void Table_clear_table_on_remove_observer(void) {
-     ecs_world_t *world = ecs_mini();
+    ecs_world_t *world = ecs_mini();
 
     ECS_COMPONENT(world, Position);
 
@@ -793,3 +793,38 @@ void Table_clear_table_on_remove_observer(void) {
 
     ecs_fini(world);
 }
+
+void Table_65_records_w_tgt(void) {
+    ecs_world_t *world = ecs_mini();
+
+    ECS_TAG(world, RelA);
+    ECS_TAG(world, TgtA);
+
+    ECS_TAG(world, RelB);
+    ECS_TAG(world, TgtB);
+
+    ecs_id_t *ids = ecs_os_malloc_n(ecs_id_t, 39);
+    int32_t i;
+
+    for (i = 0; i < 19; i ++) {
+        ids[i] = ecs_new(world);
+    }
+
+    for (; i < 37; i ++) {
+        ids[i] = ECS_AUTO_OVERRIDE | ecs_new(world);
+    }
+
+    ids[i ++] = ecs_pair(RelA, TgtA);
+    ids[i ++] = ecs_pair(RelB, TgtB);
+
+    ecs_table_t *table = ecs_table_find(world, ids, i);
+    test_assert(table != NULL);
+
+    for (i = 0; i < 39; i ++) {
+        test_assert(ecs_table_has_id(world, table, ids[i]));
+    }
+
+    ecs_os_free(ids);
+
+    ecs_fini(world);
+}

test/core/src/main.c

@@ -2324,6 +2324,7 @@ void Table_clear_table_check_size(void);
 void Table_clear_table_twice_check_size(void);
 void Table_clear_table_on_remove_hooks(void);
 void Table_clear_table_on_remove_observer(void);
+void Table_65_records_w_tgt(void);
 
 // Testsuite 'Poly'
 void Poly_on_set_poly_observer(void);
@@ -11373,6 +11374,10 @@ bake_test_case Table_testcases[] = {
     {
         "clear_table_on_remove_observer",
         Table_clear_table_on_remove_observer
+    },
+    {
+        "65_records_w_tgt",
+        Table_65_records_w_tgt
     }
 };
 
@@ -11821,7 +11826,7 @@ static bake_test_suite suites[] = {
         "Table",
         NULL,
         NULL,
-        30,
+        31,
         Table_testcases
     },
     {