Fails gracefully when allocation fails during array/map construction

PJK · Sep 29, 2022 · 41fab92 · 41fab92
1 parent e87d571
commit 41fab92
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 10 deletions.
diff --git a/src/cbor/internal/builder_callbacks.c b/src/cbor/internal/builder_callbacks.c
@@ -9,6 +9,7 @@
 #include <string.h>
 #include "../arrays.h"
 #include "../bytestrings.h"
+#include "../common.h"
 #include "../floats_ctrls.h"
 #include "../ints.h"
 #include "../maps.h"
@@ -32,17 +33,26 @@ void _cbor_builder_append(cbor_item_t *item,
            * syntax error when decoded.
            */
           assert(ctx->stack->top->subitems > 0);
-          cbor_array_push(ctx->stack->top->item, item);
-          ctx->stack->top->subitems--;
-          if (ctx->stack->top->subitems == 0) {
-            cbor_item_t *stack_item = ctx->stack->top->item;
-            _cbor_stack_pop(ctx->stack);
-            _cbor_builder_append(stack_item, ctx);
+          if (!cbor_array_push(ctx->stack->top->item, item)) {
+            ctx->creation_failed = true;
+            cbor_decref(&item);
+            break;
+          } else {
+            ctx->stack->top->subitems--;
+            if (ctx->stack->top->subitems == 0) {
+              cbor_item_t *stack_item = ctx->stack->top->item;
+              _cbor_stack_pop(ctx->stack);
+              _cbor_builder_append(stack_item, ctx);
+            }
           }
           cbor_decref(&item);
         } else {
           /* Indefinite array, don't bother with subitems */
-          cbor_array_push(ctx->stack->top->item, item);
+          if (!cbor_array_push(ctx->stack->top->item, item)) {
+            ctx->creation_failed = true;
+            cbor_decref(&item);
+            break;
+          }
           cbor_decref(&item);
         }
         break;
@@ -52,11 +62,20 @@ void _cbor_builder_append(cbor_item_t *item,
          * indefinite items */
         if (ctx->stack->top->subitems % 2) {
           /* Odd record, this is a value */
-          _cbor_map_add_value(ctx->stack->top->item, cbor_move(item));
+          if (!_cbor_map_add_value(ctx->stack->top->item, item)) {
+            ctx->creation_failed = true;
+            cbor_decref(&item);
+            break;
+          }
         } else {
           /* Even record, this is a key */
-          _cbor_map_add_key(ctx->stack->top->item, cbor_move(item));
+          if (!_cbor_map_add_key(ctx->stack->top->item, item)) {
+            ctx->creation_failed = true;
+            cbor_decref(&item);
+            break;
+          }
         }
+        cbor_decref(&item);
         if (cbor_map_is_definite(ctx->stack->top->item)) {
           ctx->stack->top->subitems--;
           if (ctx->stack->top->subitems == 0) {

diff --git a/test/memory_allocation_test.c b/test/memory_allocation_test.c
@@ -14,7 +14,7 @@
 #include "cbor.h"
 
 // This test simulates cases when malloc unexpectedly fails and leaves a
-// possibly partially constructed object behind. It this is especially useful
+// possibly partially constructed object behind. It is especially useful
 // in conjunction with the memory correctness check.
 //
 // WARNING: The test only works with CBOR_CUSTOM_ALLOC
@@ -235,6 +235,20 @@ static void test_array_push(void **_CBOR_UNUSED(_state)) {
       4, MALLOC, MALLOC, MALLOC, REALLOC_FAIL);
 }
 
+static unsigned char simple_indef_array[] = {0x9F, 0x01, 0x02, 0xFF};
+static void test_indef_array_decode(void **_CBOR_UNUSED(_state)) {
+  WITH_MOCK_MALLOC(
+      {
+        cbor_item_t *array;
+        struct cbor_load_result res;
+        array = cbor_load(simple_indef_array, 4, &res);
+
+        assert_null(array);
+        assert_int_equal(res.error.code, CBOR_ERR_MEMERROR);
+      },
+      4, MALLOC, MALLOC, MALLOC, REALLOC_FAIL);
+}
+
 static void test_map_add(void **_CBOR_UNUSED(_state)) {
   WITH_MOCK_MALLOC(
       {
@@ -254,6 +268,20 @@ static void test_map_add(void **_CBOR_UNUSED(_state)) {
       4, MALLOC, MALLOC, MALLOC, REALLOC_FAIL);
 }
 
+static unsigned char simple_indef_map[] = {0xBF, 0x01, 0x02, 0x03, 0x04, 0xFF};
+static void test_indef_map_decode(void **_CBOR_UNUSED(_state)) {
+  WITH_MOCK_MALLOC(
+      {
+        cbor_item_t *map;
+        struct cbor_load_result res;
+        map = cbor_load(simple_indef_map, 6, &res);
+
+        assert_null(map);
+        assert_int_equal(res.error.code, CBOR_ERR_MEMERROR);
+      },
+      4, MALLOC, MALLOC, MALLOC, REALLOC_FAIL);
+}
+
 int main(void) {
   cbor_set_allocs(instrumented_malloc, instrumented_realloc, free);
 
@@ -270,7 +298,9 @@ int main(void) {
       cmocka_unit_test(test_bytestring_add_chunk),
       cmocka_unit_test(test_string_add_chunk),
       cmocka_unit_test(test_array_push),
+      cmocka_unit_test(test_indef_array_decode),
       cmocka_unit_test(test_map_add),
+      cmocka_unit_test(test_indef_map_decode),
   };
 
   return cmocka_run_group_tests(tests, NULL, NULL);