commit 9123e5b11f859249f97770572e3e02d23036ab08
author David Navarro <dnav@users.noreply.github.com> Mon Mar 28 16:04:03 2022 +0200
committer GitHub <noreply@github.com> Mon Mar 28 07:04:03 2022 -0700
tree 336aa943ef0b8b6de70dfeb02183290092139a59
parent 7278700c32ec763b56efa1f5995c120c8af0140e

Handle zero-length chunks in indefinite-length strings

Previously this would generate a QCBOR_ERR_STRING_ALLOCATE error.  There is no security issue or attack vector here. QCBOR just errored out on a zero-length string chunk when it should not have.  Zero-length string chunks are explicitly allowed in RFC 8949

Thanks David for the catch and the fix!


* Fix decoding of an indefinite-length string with a zero-length first chunk. (#134)

Signed-off-by: David Navarro <david.navarro@ioterop.com>

* Add an unit test for #134.

Signed-off-by: David Navarro <david.navarro@ioterop.com>

Co-authored-by: David Navarro <david.navarro@ioterop.com>

src/qcbor_decode.c

diff --git a/src/qcbor_decode.c b/src/qcbor_decode.c
index 7ca2b81..8217073 100644
--- a/src/qcbor_decode.c
+++ b/src/qcbor_decode.c
@@ -1303,21 +1303,23 @@
          break;
       }
 
-      /* The first time throurgh FullString.ptr is NULL and this is
-       * equivalent to StringAllocator_Allocate(). Subsequently it is
-       * not NULL and a reallocation happens.
-       */
-      UsefulBuf NewMem = StringAllocator_Reallocate(pAllocator,
-                                                    FullString.ptr,
-                                                    FullString.len + StringChunkItem.val.string.len);
+      if (StringChunkItem.val.string.len > 0) {
+         /* The first time throurgh FullString.ptr is NULL and this is
+          * equivalent to StringAllocator_Allocate(). Subsequently it is
+          * not NULL and a reallocation happens.
+          */
+         UsefulBuf NewMem = StringAllocator_Reallocate(pAllocator,
+                                                       FullString.ptr,
+                                                       FullString.len + StringChunkItem.val.string.len);
 
-      if(UsefulBuf_IsNULL(NewMem)) {
-         uReturn = QCBOR_ERR_STRING_ALLOCATE;
-         break;
+         if(UsefulBuf_IsNULL(NewMem)) {
+            uReturn = QCBOR_ERR_STRING_ALLOCATE;
+            break;
+         }
+ 
+         /* Copy new string chunk to the end of accumulated string */
+         FullString = UsefulBuf_CopyOffset(NewMem, FullString.len, StringChunkItem.val.string);
       }
-
-      /* Copy new string chunk to the end of accumulated string */
-      FullString = UsefulBuf_CopyOffset(NewMem, FullString.len, StringChunkItem.val.string);
    }
 
    if(uReturn != QCBOR_SUCCESS && !UsefulBuf_IsNULLC(FullString)) {