laurencelundblade / QCBOR

Comprehensive, powerful, commercial-quality CBOR encoder/ decoder that is still suited for small devices.
Other
182 stars 47 forks source link

Fail to decode an indefinite-length string with a zero-length first chunk. #134

Closed dnav closed 2 years ago

dnav commented 2 years ago

When decoding the following CBOR:

5F     # bytes(*)
   40  # bytes(0)
       # ""
   FF  # primitive(*)

QCBORDecode_GetNext() returns a QCBOR_ERR_STRING_ALLOCATE error.

Bug seems to be in QCBORDecode_GetNextFullString() in file qcbor_decode.c at line 1310:

      /* 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;
      }

As this is the first chunk, FullString.ptr is nil and FullString.len is 0. Since StringChunkItem.val.string.len is also 0, the allocator is called with a nil pOldMem and a uNewSize of 0. The allocator then returns NULLUsefulBuf which is interpreted as an error.

Test Code

const uint8_t spTestIssueInput[] = { 0x5F, 0x40, 0xFF };

int32_t CBORTestIssue()
{
   QCBORDecodeContext DCtx;
   QCBORItem          Item;
   QCBORError         uCBORError;

   QCBORDecode_Init(&DCtx,
                    UsefulBuf_FROM_BYTE_ARRAY_LITERAL(spTestIssueInput),
                    QCBOR_DECODE_MODE_NORMAL);

   UsefulBuf_MAKE_STACK_UB(StringBuf, 200);
   QCBORDecode_SetMemPool(&DCtx, StringBuf, false);

   do {
      uCBORError = QCBORDecode_GetNext(&DCtx, &Item);
   } while (QCBOR_SUCCESS == uCBORError);

   if (QCBOR_SUCCESS == uCBORError)
   {
      uCBORError = QCBORDecode_Finish(&DCtx);
   }

   return uCBORError;
}
laurencelundblade commented 2 years ago

Fixed in #135