QCBOR: Improve handling of end of data and error; add indefinite length encoding
* Minor improvements / fixes in run_test framework
* Add CBOR indefinite length encoding
* Recheck pointer math in UsefulBuf and remove "TODO"
* Better error handling of not-well-formed CBOR when decoding
* Better handling of end of data when decoding
* Better handling of encode error when out of space in output buffer
Change-Id: Ib8dc2af95bc533b7905648d8f8c3b1bf1c42ba44
Signed-off-by: Laurence Lundblade <lgl@securitytheory.com>
diff --git a/lib/ext/qcbor/src/UsefulBuf.c b/lib/ext/qcbor/src/UsefulBuf.c
index f13dad2..0c336b8 100644
--- a/lib/ext/qcbor/src/UsefulBuf.c
+++ b/lib/ext/qcbor/src/UsefulBuf.c
@@ -41,6 +41,7 @@
when who what, where, why
-------- ---- ---------------------------------------------------
+ 11/08/2019 llundblade Re check pointer math and update comments
3/6/2019 llundblade Add UsefulBuf_IsValue()
09/07/17 llundbla Fix critical bug in UsefulBuf_Find() -- a read off
the end of memory when the bytes to find is longer
@@ -222,7 +223,7 @@
}
/* 1. Will it fit? */
- // WillItFit() is the same as: NewData.len <= (me->size - me->data_len)
+ // WillItFit() is the same as: NewData.len <= (me->UB.len - me->data_len)
// Check #1 makes sure subtraction in RoomLeft will not wrap around
if(! UsefulOutBuf_WillItFit(pMe, NewData.len)) { // Check #2
// The new data will not fit into the the buffer.
@@ -231,7 +232,9 @@
}
/* 2. Check the Insertion Position */
- // This, with Check #1, also confirms that uInsertionPos <= me->data_len
+ // This, with Check #1, also confirms that uInsertionPos <= me->data_len and
+ // that uInsertionPos + pMe->UB.ptr will not wrap around the end of the
+ // address space.
if(uInsertionPos > pMe->data_len) { // Check #3
// Off the end of the valid data in the buffer.
pMe->err = 1;
@@ -245,6 +248,7 @@
if(uNumBytesToMove && pMe->UB.ptr) {
// To know memmove won't go off end of destination, see PtrMath #4
+ // Use memove because it handles overlapping buffers
memmove(pDestinationOfMove, pSourceOfMove, uNumBytesToMove);
}
@@ -254,7 +258,7 @@
// To know memmove won't go off end of destination, see PtrMath #6
memmove(pInsertionPoint, NewData.ptr, NewData.len);
}
- pMe->data_len += NewData.len ;
+ pMe->data_len += NewData.len;
}
@@ -269,9 +273,9 @@
PtrMath #2 will never wrap around under because
Check #3 makes sure uInsertionPos is less than me->data_len
- PtrMath #3 will never wrap around over because todo
- PtrMath #1 is checked resulting in pSourceOfMove being between me->UB.ptr and a maximum valid ptr
- Check #2 that NewData.len will fit
+ PtrMath #3 will never wrap around over because
+ PtrMath #1 is checked resulting in pSourceOfMove being between me->UB.ptr and me->UB.ptr + me->data_len
+ Check #2 that NewData.len will fit in the unused space left in me->UB
PtrMath #4 will never wrap under because
Calculation for extent or memmove is uRoomInDestination = me->UB.len - (uInsertionPos + NewData.len)
diff --git a/lib/ext/qcbor/src/qcbor_decode.c b/lib/ext/qcbor/src/qcbor_decode.c
index fa89b09..36a9d11 100644
--- a/lib/ext/qcbor/src/qcbor_decode.c
+++ b/lib/ext/qcbor/src/qcbor_decode.c
@@ -42,6 +42,10 @@
when who what, where, why
-------- ---- ---------------------------------------------------
+ 11/07/19 llundblade Fix long long conversion to double compiler warning
+ 09/07/19 llundblade Fix bug decoding empty arrays and maps
+ 07/31/19 llundblade Decode error fixes for some not-well-formed CBOR
+ 07/31/19 llundblade New error code for better end of data handling
02/17/19 llundblade Fixed: QCBORItem.u{Data|Label}Alloc when bAllStrings set
02/16/19 llundblade Redesign MemPool to fix memory access alignment bug
01/10/19 llundblade Clever type and argument decoder is 250 bytes smaller
@@ -127,28 +131,27 @@
// Called on every single item except breaks including the opening of a map/array
inline static void DecodeNesting_DecrementCount(QCBORDecodeNesting *pNesting)
{
- if(!DecodeNesting_IsNested(pNesting)) {
- // at top level where there is no tracking
- return;
- }
+ while(DecodeNesting_IsNested(pNesting)) {
+ // Not at the top level, so there is decrementing to be done.
- if(DecodeNesting_IsIndefiniteLength(pNesting)) {
- // There is no count for indefinite length arrays/maps
- return;
- }
-
- // Decrement the count of items in this array/map
- pNesting->pCurrent->uCount--;
-
- // Pop up nesting levels if the counts at the levels are zero
- while(DecodeNesting_IsNested(pNesting) && 0 == pNesting->pCurrent->uCount) {
- pNesting->pCurrent--;
if(!DecodeNesting_IsIndefiniteLength(pNesting)) {
+ // Decrement the current nesting level if it is not indefinite.
pNesting->pCurrent->uCount--;
}
+
+ if(pNesting->pCurrent->uCount != 0) {
+ // Did not close out an array or map, so nothing further
+ break;
+ }
+
+ // Closed out an array or map so level up
+ pNesting->pCurrent--;
+
+ // Continue with loop to see if closing out this doesn't close out more
}
}
+
// Called on every map/array
inline static QCBORError DecodeNesting_Descend(QCBORDecodeNesting *pNesting, QCBORItem *pItem)
{
@@ -485,7 +488,7 @@
} else {
// C can't represent a negative integer in this range
- // so it is an error. todo -- test this condition
+ // so it is an error.
nReturn = QCBOR_ERR_INT_OVERFLOW;
}
}
@@ -536,11 +539,7 @@
pDecodedItem->uDataType = uAdditionalInfo;
switch(uAdditionalInfo) {
- case ADDINFO_RESERVED1: // 28
- case ADDINFO_RESERVED2: // 29
- case ADDINFO_RESERVED3: // 30
- nReturn = QCBOR_ERR_UNSUPPORTED;
- break;
+ // No check for ADDINFO_RESERVED1 - ADDINFO_RESERVED3 as it is caught before this is called.
case HALF_PREC_FLOAT:
pDecodedItem->val.dfnum = IEEE754_HalfToDouble((uint16_t)uNumber);
@@ -660,7 +659,7 @@
/*
The epoch formatted date. Turns lots of different forms of encoding date into uniform one
*/
-static QCBORError DecodeDateEpoch(QCBORItem *pDecodedItem)
+static int DecodeDateEpoch(QCBORItem *pDecodedItem)
{
// Stack usage: 1
QCBORError nReturn = QCBOR_SUCCESS;
@@ -683,13 +682,33 @@
case QCBOR_TYPE_DOUBLE:
{
+ // This comparison needs to be done as a float before
+ // conversion to an int64_t to be able to detect doubles
+ // that are too large to fit into an int64_t. A double
+ // has 52 bits of preceision. An int64_t has 63. Casting
+ // INT64_MAX to a double actually causes a round up which
+ // is bad and wrong for the comparison because it will
+ // allow conversion of doubles that can't fit into a
+ // uint64_t. To remedy this INT64_MAX - 0x7ff is used as
+ // the cutoff point as if that rounds up in conversion to
+ // double it will still be less than INT64_MAX. 0x7ff is
+ // picked because it has 11 bits set.
+ //
+ // INT64_MAX seconds is on the order of 10 billion years,
+ // and the earth is less than 5 billion years old, so for
+ // most uses this conversion error won't occur even though
+ // doubles can go much larger.
+ //
+ // Without the 0x7ff there is a ~30 minute range of time
+ // values 10 billion years in the past and in the future
+ // where this this code would go wrong.
const double d = pDecodedItem->val.dfnum;
- if(d > INT64_MAX) {
+ if(d > (double)(INT64_MAX - 0x7ff)) {
nReturn = QCBOR_ERR_DATE_OVERFLOW;
goto Done;
}
- pDecodedItem->val.epochDate.nSeconds = (int64_t) d; // Float to integer conversion happening here.
- pDecodedItem->val.epochDate.fSecondsFraction = d - pDecodedItem->val.epochDate.nSeconds;
+ pDecodedItem->val.epochDate.nSeconds = (int64_t)d;
+ pDecodedItem->val.epochDate.fSecondsFraction = d - (double)pDecodedItem->val.epochDate.nSeconds;
}
break;
@@ -735,21 +754,26 @@
uint64_t uNumber;
uint8_t uAdditionalInfo;
+ memset(pDecodedItem, 0, sizeof(QCBORItem));
+
nReturn = DecodeTypeAndNumber(pUInBuf, &uMajorType, &uNumber, &uAdditionalInfo);
// Error out here if we got into trouble on the type and number.
// The code after this will not work if the type and number is not good.
- if(nReturn)
+ if(nReturn) {
goto Done;
-
- memset(pDecodedItem, 0, sizeof(QCBORItem));
+ }
// At this point the major type and the value are valid. We've got the type and the number that
// starts every CBOR data item.
switch (uMajorType) {
case CBOR_MAJOR_TYPE_POSITIVE_INT: // Major type 0
case CBOR_MAJOR_TYPE_NEGATIVE_INT: // Major type 1
- nReturn = DecodeInteger(uMajorType, uNumber, pDecodedItem);
+ if(uAdditionalInfo == LEN_IS_INDEFINITE) {
+ nReturn = QCBOR_ERR_BAD_INT;
+ } else {
+ nReturn = DecodeInteger(uMajorType, uNumber, pDecodedItem);
+ }
break;
case CBOR_MAJOR_TYPE_BYTE_STRING: // Major type 2
@@ -778,8 +802,12 @@
break;
case CBOR_MAJOR_TYPE_OPTIONAL: // Major type 6, optional prepended tags
- pDecodedItem->val.uTagV = uNumber;
- pDecodedItem->uDataType = QCBOR_TYPE_OPTTAG;
+ if(uAdditionalInfo == LEN_IS_INDEFINITE) {
+ nReturn = QCBOR_ERR_BAD_INT;
+ } else {
+ pDecodedItem->val.uTagV = uNumber;
+ pDecodedItem->uDataType = QCBOR_TYPE_OPTTAG;
+ }
break;
case CBOR_MAJOR_TYPE_SIMPLE: // Major type 7, float, double, true, false, null...
@@ -864,7 +892,8 @@
// Match data type of chunk to type at beginning.
// Also catches error of other non-string types that don't belong.
- if(StringChunkItem.uDataType != uStringType) {
+ // Also catches indefinite length strings inside indefinite length strings
+ if(StringChunkItem.uDataType != uStringType || StringChunkItem.val.string.len == SIZE_MAX) {
nReturn = QCBOR_ERR_INDEFINITE_STRING_CHUNK;
break;
}
@@ -1060,6 +1089,12 @@
// All the CBOR parsing work is here and in subordinate calls.
QCBORError nReturn;
+ // Check if there are an
+ if(UsefulInputBuf_BytesUnconsumed(&(me->InBuf)) == 0 && !DecodeNesting_IsNested(&(me->nesting))) {
+ nReturn = QCBOR_ERR_NO_MORE_ITEMS;
+ goto Done;
+ }
+
nReturn = GetNext_MapEntry(me, pDecodedItem, pTags);
if(nReturn) {
goto Done;
@@ -1084,7 +1119,10 @@
// Maps and arrays do count in as items in the map/array that encloses
// them so a decrement needs to be done for them too, but that is done
// only when all the items in them have been processed, not when they
- // are opened.
+ // are opened with the exception of an empty map or array.
+ if(pDecodedItem->val.uCount == 0) {
+ DecodeNesting_DecrementCount(&(me->nesting));
+ }
} else {
// Decrement the count of items in the enclosing map/array
// If the count in the enclosing map/array goes to zero, that
@@ -1130,6 +1168,10 @@
pDecodedItem->uNextNestLevel = DecodeNesting_GetLevel(&(me->nesting));
Done:
+ if(nReturn != QCBOR_SUCCESS) {
+ // Make sure uDataType and uLabelType are QCBOR_TYPE_NONE
+ memset(pDecodedItem, 0, sizeof(QCBORItem));
+ }
return nReturn;
}
@@ -1199,7 +1241,7 @@
*/
QCBORError QCBORDecode_Finish(QCBORDecodeContext *me)
{
- QCBORError nReturn = QCBOR_SUCCESS;
+ int nReturn = QCBOR_SUCCESS;
// Error out if all the maps/arrays are not closed out
if(DecodeNesting_IsNested(&(me->nesting))) {
diff --git a/lib/ext/qcbor/src/qcbor_encode.c b/lib/ext/qcbor/src/qcbor_encode.c
index 1155cda..28fb225 100644
--- a/lib/ext/qcbor/src/qcbor_encode.c
+++ b/lib/ext/qcbor/src/qcbor_encode.c
@@ -42,6 +42,8 @@
when who what, where, why
-------- ---- ---------------------------------------------------
+ 8/7/19 llundblade Prevent encoding simple type reserved values 24..31
+ 7/25/19 janjongboom Add indefinite length encoding for maps and arrays
4/6/19 llundblade Wrapped bstr returned now includes the wrapping bstr
12/30/18 llundblade Small efficient clever encode of type & argument.
11/29/18 llundblade Rework to simpler handling of tags and labels.
@@ -188,7 +190,7 @@
structures like array/map nesting resulting in some stack memory
savings.
- Errors returned here fall into two categories:
+ Errors returned here fall into three categories:
Sizes
QCBOR_ERR_BUFFER_TOO_LARGE -- Encoded output exceeded UINT32_MAX
@@ -201,6 +203,9 @@
QCBOR_ERR_TOO_MANY_CLOSES -- more close calls than opens
QCBOR_ERR_CLOSE_MISMATCH -- Type of close does not match open
QCBOR_ERR_ARRAY_OR_MAP_STILL_OPEN -- Finish called without enough closes
+
+ Would generate not-well-formed CBOR
+ QCBOR_ERR_UNSUPPORTED -- Simple type between 24 and 31
*/
@@ -300,9 +305,18 @@
// This is the 5 bits in the initial byte that is not the major type
uint8_t uAdditionalInfo;
- if(uNumber < CBOR_TWENTY_FOUR && nMinLen == 0) {
+ if (uMajorType == CBOR_MAJOR_NONE_TYPE_ARRAY_INDEFINITE_LEN) {
+ uMajorType = CBOR_MAJOR_TYPE_ARRAY;
+ uAdditionalInfo = LEN_IS_INDEFINITE;
+ } else if (uMajorType == CBOR_MAJOR_NONE_TYPE_MAP_INDEFINITE_LEN) {
+ uMajorType = CBOR_MAJOR_TYPE_MAP;
+ uAdditionalInfo = LEN_IS_INDEFINITE;
+ } else if (uNumber < CBOR_TWENTY_FOUR && nMinLen == 0) {
// Simple case where argument is < 24
uAdditionalInfo = uNumber;
+ } else if (uMajorType == CBOR_MAJOR_TYPE_SIMPLE && uNumber == CBOR_SIMPLE_BREAK) {
+ // Break statement can be encoded in single byte too (0xff)
+ uAdditionalInfo = uNumber;
} else {
/*
Encode argument in 1,2,4 or 8 bytes. Outer loop
@@ -453,18 +467,22 @@
void QCBOREncode_AddType7(QCBOREncodeContext *me, size_t uSize, uint64_t uNum)
{
if(me->uError == QCBOR_SUCCESS) {
- // This function call takes care of endian swapping for the float / double
- InsertEncodedTypeAndNumber(me,
- // The major type for floats and doubles
- CBOR_MAJOR_TYPE_SIMPLE,
- // size makes sure floats with zeros encode correctly
- (int)uSize,
- // Bytes of the floating point number as a uint
- uNum,
- // end position because this is append
- UsefulOutBuf_GetEndPosition(&(me->OutBuf)));
+ if(uNum >= CBOR_SIMPLEV_RESERVED_START && uNum <= CBOR_SIMPLEV_RESERVED_END) {
+ me->uError = QCBOR_ERR_UNSUPPORTED;
+ } else {
+ // This function call takes care of endian swapping for the float / double
+ InsertEncodedTypeAndNumber(me,
+ // The major type for floats and doubles
+ CBOR_MAJOR_TYPE_SIMPLE,
+ // size makes sure floats with zeros encode correctly
+ (int)uSize,
+ // Bytes of the floating point number as a uint
+ uNum,
+ // end position because this is append
+ UsefulOutBuf_GetEndPosition(&(me->OutBuf)));
- me->uError = Nesting_Increment(&(me->nesting));
+ me->uError = Nesting_Increment(&(me->nesting));
+ }
}
}
@@ -515,6 +533,19 @@
}
}
+/*
+ Semi-public function. It is exposed to user of the interface,
+ but they will usually call one of the inline wrappers rather than this.
+
+ See header qcbor.h
+*/
+void QCBOREncode_OpenMapOrArrayIndefiniteLength(QCBOREncodeContext *me, uint8_t uMajorType)
+{
+ // insert the indefinite length marker (0x9f for arrays, 0xbf for maps)
+ InsertEncodedTypeAndNumber(me, uMajorType, 0, 0, UsefulOutBuf_GetEndPosition(&(me->OutBuf)));
+
+ QCBOREncode_OpenMapOrArray(me, uMajorType);
+}
/*
Public functions for closing arrays and maps. See header qcbor.h
@@ -568,7 +599,35 @@
}
}
+/*
+ Public functions for closing arrays and maps. See header qcbor.h
+ */
+void QCBOREncode_CloseMapOrArrayIndefiniteLength(QCBOREncodeContext *me, uint8_t uMajorType, UsefulBufC *pWrappedCBOR)
+{
+ if(me->uError == QCBOR_SUCCESS) {
+ if(!Nesting_IsInNest(&(me->nesting))) {
+ me->uError = QCBOR_ERR_TOO_MANY_CLOSES;
+ } else if(Nesting_GetMajorType(&(me->nesting)) != uMajorType) {
+ me->uError = QCBOR_ERR_CLOSE_MISMATCH;
+ } else {
+ // insert the break marker (0xff for both arrays and maps)
+ InsertEncodedTypeAndNumber(me, CBOR_MAJOR_TYPE_SIMPLE, 0, CBOR_SIMPLE_BREAK, UsefulOutBuf_GetEndPosition(&(me->OutBuf)));
+ // Return pointer and length to the enclosed encoded CBOR. The intended
+ // use is for it to be hashed (e.g., SHA-256) in a COSE implementation.
+ // This must be used right away, as the pointer and length go invalid
+ // on any subsequent calls to this function because there might be calls to
+ // InsertEncodedTypeAndNumber() that slides data to the right.
+ if(pWrappedCBOR) {
+ const UsefulBufC PartialResult = UsefulOutBuf_OutUBuf(&(me->OutBuf));
+ *pWrappedCBOR = UsefulBuf_Tail(PartialResult, UsefulOutBuf_GetEndPosition(&(me->OutBuf)));
+ }
+
+ // Decrease nesting level
+ Nesting_Decrease(&(me->nesting));
+ }
+ }
+}
/*
@@ -576,7 +635,7 @@
*/
QCBORError QCBOREncode_Finish(QCBOREncodeContext *me, UsefulBufC *pEncodedCBOR)
{
- QCBORError uReturn = (QCBORError) me->uError;
+ QCBORError uReturn = QCBOREncode_GetErrorState(me);
if(uReturn != QCBOR_SUCCESS) {
goto Done;
@@ -587,17 +646,6 @@
goto Done;
}
- if(UsefulOutBuf_GetError(&(me->OutBuf))) {
- // Items didn't fit in the buffer.
- // This check catches this condition for all the appends and inserts
- // so checks aren't needed when the appends and inserts are performed.
- // And of course UsefulBuf will never overrun the input buffer given
- // to it. No complex analysis of the error handling in this file is
- // needed to know that is true. Just read the UsefulBuf code.
- uReturn = QCBOR_ERR_BUFFER_TOO_SMALL;
- goto Done;
- }
-
*pEncodedCBOR = UsefulOutBuf_OutUBuf(&(me->OutBuf));
Done:
@@ -605,6 +653,7 @@
}
+
/*
Public functions to finish and get the encoded result. See header qcbor.h
*/
@@ -662,4 +711,3 @@
instance can be removed, saving some code.
*/
-