Fix memory allocation error handling in cbor_copy

CHANGELOG.md

@@ -14,6 +14,9 @@ Next
 - [Fix a memory leak when the allocator fails when adding chunks to indefinite strings](https://github.com/PJK/libcbor/pull/246)
 - Potentially BUILD BREAKING: [Add nodiscard attributes to most functions](https://github.com/PJK/libcbor/pull/248)
   - **Warning**: This may cause new build warnings and (in rare cases, depending on your configuration) errors
+- BREAKING: [Fix `cbor_copy` leaking memory and creating invalid items when the allocator fails](https://github.com/PJK/libcbor/pull/249).
+  - Previously, the failures were not handled in the interface. Now, `cbor_copy` may return `NULL` upon failure; clients should check the return value
+- [Fix `cbor_build_tag` illegal memory behavior when the allocator fails](https://github.com/PJK/libcbor/pull/249)
 
 
 0.9.0 (2021-11-14)

src/cbor.c

@@ -167,10 +167,24 @@ cbor_item_t *cbor_copy(cbor_item_t *item) {
                                      cbor_bytestring_length(item));
       } else {
         cbor_item_t *res = cbor_new_indefinite_bytestring();
-        for (size_t i = 0; i < cbor_bytestring_chunk_count(item); i++)
-          cbor_bytestring_add_chunk(
-              res,
-              cbor_move(cbor_copy(cbor_bytestring_chunks_handle(item)[i])));
+        if (res == NULL) {
+          return NULL;
+        }
+
+        for (size_t i = 0; i < cbor_bytestring_chunk_count(item); i++) {
+          cbor_item_t *chunk_copy =
+              cbor_copy(cbor_bytestring_chunks_handle(item)[i]);
+          if (chunk_copy == NULL) {
+            cbor_decref(&res);
+            return NULL;
+          }
+          if (!cbor_bytestring_add_chunk(res, chunk_copy)) {
+            cbor_decref(&chunk_copy);
+            cbor_decref(&res);
+            return NULL;
+          }
+          cbor_decref(&chunk_copy);
+        }
         return res;
       }
     case CBOR_TYPE_STRING:
@@ -179,41 +193,97 @@ cbor_item_t *cbor_copy(cbor_item_t *item) {
                                   cbor_string_length(item));
       } else {
         cbor_item_t *res = cbor_new_indefinite_string();
-        for (size_t i = 0; i < cbor_string_chunk_count(item); i++)
-          cbor_string_add_chunk(
-              res, cbor_move(cbor_copy(cbor_string_chunks_handle(item)[i])));
+        if (res == NULL) {
+          return NULL;
+        }
+
+        for (size_t i = 0; i < cbor_string_chunk_count(item); i++) {
+          cbor_item_t *chunk_copy =
+              cbor_copy(cbor_string_chunks_handle(item)[i]);
+          if (chunk_copy == NULL) {
+            cbor_decref(&res);
+            return NULL;
+          }
+          if (!cbor_string_add_chunk(res, chunk_copy)) {
+            cbor_decref(&chunk_copy);
+            cbor_decref(&res);
+            return NULL;
+          }
+          cbor_decref(&chunk_copy);
+        }
         return res;
       }
     case CBOR_TYPE_ARRAY: {
       cbor_item_t *res;
-      if (cbor_array_is_definite(item))
+      if (cbor_array_is_definite(item)) {
         res = cbor_new_definite_array(cbor_array_size(item));
-      else
+      } else {
         res = cbor_new_indefinite_array();
+      }
+      if (res == NULL) {
+        return NULL;
+      }
 
-      for (size_t i = 0; i < cbor_array_size(item); i++)
-        cbor_array_push(
-            res, cbor_move(cbor_copy(cbor_move(cbor_array_get(item, i)))));
+      for (size_t i = 0; i < cbor_array_size(item); i++) {
+        cbor_item_t *entry_copy = cbor_copy(cbor_move(cbor_array_get(item, i)));
+        if (entry_copy == NULL) {
+          cbor_decref(&res);
+          return NULL;
+        }
+        if (!cbor_array_push(res, entry_copy)) {
+          cbor_decref(&entry_copy);
+          cbor_decref(&res);
+          return NULL;
+        }
+        cbor_decref(&entry_copy);
+      }
       return res;
     }
     case CBOR_TYPE_MAP: {
       cbor_item_t *res;
-      if (cbor_map_is_definite(item))
+      if (cbor_map_is_definite(item)) {
         res = cbor_new_definite_map(cbor_map_size(item));
-      else
+      } else {
         res = cbor_new_indefinite_map();
+      }
+      if (res == NULL) {
+        return NULL;
+      }
 
       struct cbor_pair *it = cbor_map_handle(item);
-      for (size_t i = 0; i < cbor_map_size(item); i++)
-        cbor_map_add(res, (struct cbor_pair){
-                              .key = cbor_move(cbor_copy(it[i].key)),
-                              .value = cbor_move(cbor_copy(it[i].value))});
+      for (size_t i = 0; i < cbor_map_size(item); i++) {
+        cbor_item_t *key_copy = cbor_copy(it[i].key);
+        if (key_copy == NULL) {
+          cbor_decref(&res);
+          return NULL;
+        }
+        cbor_item_t *value_copy = cbor_copy(it[i].value);
+        if (value_copy == NULL) {
+          cbor_decref(&res);
+          cbor_decref(&key_copy);
+          return NULL;
+        }
+        if (!cbor_map_add(res, (struct cbor_pair){.key = key_copy,
+                                                  .value = value_copy})) {
+          cbor_decref(&res);
+          cbor_decref(&key_copy);
+          cbor_decref(&value_copy);
+          return NULL;
+        }
+        cbor_decref(&key_copy);
+        cbor_decref(&value_copy);
+      }
       return res;
     }
-    case CBOR_TYPE_TAG:
-      return cbor_build_tag(
-          cbor_tag_value(item),
-          cbor_move(cbor_copy(cbor_move(cbor_tag_item(item)))));
+    case CBOR_TYPE_TAG: {
+      cbor_item_t *item_copy = cbor_copy(cbor_move(cbor_tag_item(item)));
+      if (item_copy == NULL) {
+        return NULL;
+      }
+      cbor_item_t *tag = cbor_build_tag(cbor_tag_value(item), item_copy);
+      cbor_decref(&item_copy);
+      return tag;
+    }
     case CBOR_TYPE_FLOAT_CTRL:
       return _cbor_copy_float_ctrl(item);
   }

src/cbor.h

@@ -46,12 +46,12 @@ extern "C" {
 _CBOR_NODISCARD CBOR_EXPORT cbor_item_t* cbor_load(
     cbor_data source, size_t source_size, struct cbor_load_result* result);
 
-/** Deep copy of an item
+/** Take a deep copy of an item
  *
  * All the reference counts in the new structure are set to one.
  *
  * @param item[borrow] item to copy
- * @return **new** CBOR deep copy
+ * @return **new** CBOR deep copy or `NULL` on failure.
  */
 _CBOR_NODISCARD CBOR_EXPORT cbor_item_t* cbor_copy(cbor_item_t* item);
 

src/cbor/tags.c

@@ -38,6 +38,9 @@ void cbor_tag_set_item(cbor_item_t *item, cbor_item_t *tagged_item) {
 
 cbor_item_t *cbor_build_tag(uint64_t value, cbor_item_t *item) {
   cbor_item_t *res = cbor_new_tag(value);
+  if (res == NULL) {
+    return NULL;
+  }
   cbor_tag_set_item(res, item);
   return res;
 }