Encode test coverage 100%; MAX_ITEMS_IN_MAP (#256)
Just needed a few more tests to hit 100%.
The max size of maps is now QCBOR_MAX_ITEMS_IN_MAP which is half of QCBOR_MAX_ITEMS_IN_ARRAY. This limit
was enforced for encoding, but incorrectly documented as being QCBOR_MAX_ITEMS_IN_ARRAY. The new constant
makes this clear.
The max size for decoding maps is reduced by half so it is consistent for all map use, encoding and decoding and map-decoding-as-an-array. Strictly speaking this is an incompatible change, but probably of no consequence because it went from 65,534 to 32,767 and 32,767 is still very large for a map.
Tiny encoder bug was found and fixed -- maps and arrays were limited to QCBOR_MAX_ITEMS_IN_ARRAY-1 rather than QCBOR_MAX_ITEMS_IN_ARRAY.
* Encode test coverage increased to 100%
* fix limit on map max size; test array/map max size
* straggler from last ccommit
* final clean up
---------
Co-authored-by: Laurence Lundblade <lgl@securitytheory.com>
diff --git a/inc/qcbor/qcbor_common.h b/inc/qcbor/qcbor_common.h
index 43d3a3d..1ebfb7b 100644
--- a/inc/qcbor/qcbor_common.h
+++ b/inc/qcbor/qcbor_common.h
@@ -280,8 +280,9 @@
* different type than is currently open. */
QCBOR_ERR_CLOSE_MISMATCH = 5,
- /** During encoding, the array or map had too many items in it.
- * This limit @ref QCBOR_MAX_ITEMS_IN_ARRAY, typically 65,535. */
+ /** During encoding, the array or map had too many items in it. The
+ * limits are @ref QCBOR_MAX_ITEMS_IN_ARRAY and
+ * @ref QCBOR_MAX_ITEMS_IN_MAP. */
QCBOR_ERR_ARRAY_TOO_LONG = 6,
/** During encoding, more arrays or maps were closed than
@@ -368,9 +369,9 @@
QCBOR_ERR_ARRAY_DECODE_NESTING_TOO_DEEP = 41,
/** During decoding, the array or map had too many items in it.
- * This limit @ref QCBOR_MAX_ITEMS_IN_ARRAY, typically 65,534,
- * UINT16_MAX - 1. This error makes no further decoding
- * possible. */
+ * This limit is @ref QCBOR_MAX_ITEMS_IN_ARRAY (65,534) for
+ * arrays and @ref QCBOR_MAX_ITEMS_IN_MAP (32,767) for maps. This
+ * error makes no further decoding possible. */
QCBOR_ERR_ARRAY_DECODE_TOO_LONG = 42,
/** When decoding, a string's size is greater than what a size_t
@@ -568,14 +569,19 @@
/**
- * The maximum number of items in a single array or map when encoding or
- * decoding.
+ * The maximum number of items in a single array when encoding or
+ * decoding. See also @ref QCBOR_MAX_ITEMS_IN_MAP.
*/
#define QCBOR_MAX_ITEMS_IN_ARRAY (UINT16_MAX-1) /* -1 is because the
* value UINT16_MAX is
* used to indicate
* indefinite-length.
*/
+/**
+ * The maximum number of items in a single map when encoding or
+ * decoding. See also @ref QCBOR_MAX_ITEMS_IN_ARRAY.
+ */
+#define QCBOR_MAX_ITEMS_IN_MAP (QCBOR_MAX_ITEMS_IN_ARRAY/2)
#ifdef __cplusplus
diff --git a/src/qcbor_decode.c b/src/qcbor_decode.c
index 646c0d4..0d689d8 100644
--- a/src/qcbor_decode.c
+++ b/src/qcbor_decode.c
@@ -461,8 +461,8 @@
static QCBORError
DecodeNesting_DescendMapOrArray(QCBORDecodeNesting *pNesting,
- uint8_t uQCBORType,
- uint64_t uCount)
+ const uint8_t uQCBORType,
+ const uint16_t uCount)
{
QCBORError uError = QCBOR_SUCCESS;
@@ -474,21 +474,16 @@
/* Empty indefinite-length maps and arrays are handled elsewhere */
}
- /* Error out if arrays is too long to handle */
- if(uCount != QCBOR_COUNT_INDICATES_INDEFINITE_LENGTH &&
- uCount > QCBOR_MAX_ITEMS_IN_ARRAY) {
- uError = QCBOR_ERR_ARRAY_DECODE_TOO_LONG;
- goto Done;
- }
+ /* Rely on check in QCBOR_Private_DecodeArrayOrMap() for definite-length
+ * arrays and maps that are too long */
uError = DecodeNesting_Descend(pNesting, uQCBORType);
if(uError != QCBOR_SUCCESS) {
goto Done;
}
- /* Fill in the new map/array level. Check above makes casts OK. */
- pNesting->pCurrent->u.ma.uCountCursor = (uint16_t)uCount;
- pNesting->pCurrent->u.ma.uCountTotal = (uint16_t)uCount;
+ pNesting->pCurrent->u.ma.uCountCursor = uCount;
+ pNesting->pCurrent->u.ma.uCountTotal = uCount;
DecodeNesting_ClearBoundedMode(pNesting);
@@ -1042,7 +1037,7 @@
static QCBORError
QCBOR_Private_DecodeArrayOrMap(const uint8_t uMode,
const int nMajorType,
- const uint64_t uItemCount,
+ uint64_t uItemCount,
const int nAdditionalInfo,
QCBORItem *pDecodedItem)
{
@@ -1075,28 +1070,20 @@
uReturn = QCBOR_ERR_INDEF_LEN_ARRAYS_DISABLED;
#endif /* ! QCBOR_DISABLE_INDEFINITE_LENGTH_ARRAYS */
} else {
+ /* ----- Definite-length array/map ----- */
+ if(uItemCount > (nMajorType == QCBOR_TYPE_MAP ? QCBOR_MAX_ITEMS_IN_MAP : QCBOR_MAX_ITEMS_IN_ARRAY)) {
+ uReturn = QCBOR_ERR_ARRAY_DECODE_TOO_LONG;
+ } else {
#ifndef QCBOR_DISABLE_NON_INTEGER_LABELS
- if(uMode == QCBOR_DECODE_MODE_MAP_AS_ARRAY && nMajorType == QCBOR_TYPE_MAP) {
- /* ------ Definite-length map as array ------ */
-
- if(uItemCount > QCBOR_MAX_ITEMS_IN_ARRAY/2) {
- uReturn = QCBOR_ERR_ARRAY_DECODE_TOO_LONG;
- } else {
- /* cast OK because of check above */
- pDecodedItem->val.uCount = (uint16_t)uItemCount*2;
+ if(uMode == QCBOR_DECODE_MODE_MAP_AS_ARRAY && nMajorType == QCBOR_TYPE_MAP) {
+ /* ------ Map as array ------ */
+ uItemCount *= 2;
}
-
- } else
#endif /* ! QCBOR_DISABLE_NON_INTEGER_LABELS */
- {
- /* ------ Definite-length array/map ------ */
- if(uItemCount > QCBOR_MAX_ITEMS_IN_ARRAY) {
- uReturn = QCBOR_ERR_ARRAY_DECODE_TOO_LONG;
- } else {
- /* cast OK because of check above */
- pDecodedItem->val.uCount = (uint16_t)uItemCount;
- }
+
+ /* cast OK because of check above */
+ pDecodedItem->val.uCount = (uint16_t)uItemCount;
}
}
@@ -2081,8 +2068,8 @@
*/
QCBORError uDescendErr;
uDescendErr = DecodeNesting_DescendMapOrArray(&(pMe->nesting),
- pDecodedItem->uDataType,
- pDecodedItem->val.uCount);
+ pDecodedItem->uDataType,
+ pDecodedItem->val.uCount);
if(uDescendErr != QCBOR_SUCCESS) {
/* This error is probably a traversal error and it overrides
* the non-traversal error.
diff --git a/src/qcbor_encode.c b/src/qcbor_encode.c
index 3986cbb..7657303 100644
--- a/src/qcbor_encode.c
+++ b/src/qcbor_encode.c
@@ -111,7 +111,7 @@
Nesting_Increment(QCBORTrackNesting *pNesting)
{
#ifndef QCBOR_DISABLE_ENCODE_USAGE_GUARDS
- if(1 >= QCBOR_MAX_ITEMS_IN_ARRAY - pNesting->pCurrentNesting->uCount) {
+ if(pNesting->pCurrentNesting->uCount >= QCBOR_MAX_ITEMS_IN_ARRAY) {
return QCBOR_ERR_ARRAY_TOO_LONG;
}
#endif /* ! QCBOR_DISABLE_ENCODE_USAGE_GUARDS */
@@ -223,7 +223,7 @@
*
* QCBOR_DISABLE_ENCODE_USAGE_GUARDS also disables the check for more
* than QCBOR_MAX_ITEMS_IN_ARRAY in an array. Since
- * QCBOR_MAX_ITEMS_IN_ARRAY is very large (65,535) it is very unlikely
+ * QCBOR_MAX_ITEMS_IN_ARRAY is very large (65,534) it is very unlikely
* to be reached. If it is reached, the count will wrap around to zero
* and CBOR that is not well formed will be produced, but there will
* be no buffers overrun and new security issues in the code.
diff --git a/test/qcbor_decode_tests.c b/test/qcbor_decode_tests.c
index 7434f0d..9f16668 100644
--- a/test/qcbor_decode_tests.c
+++ b/test/qcbor_decode_tests.c
@@ -2297,9 +2297,61 @@
}
+/* These are just the item that open large maps and arrays, not
+ * the items in the array. This is sufficient to test the
+ * boundary condition. */
+static const uint8_t spLargeArrayFake[] = {
+ 0x99, 0xff, 0xfe};
+
+static const uint8_t spTooLargeArrayFake[] = {
+ 0x99, 0xff, 0xff};
+
+static const uint8_t spLargeMapFake[] = {
+ 0xb9, 0x7f, 0xff};
+
+static const uint8_t spTooLargeMapFake[] = {
+ 0xba, 0x00, 0x00, 0x80, 0x00};
+
int32_t ParseMapTest(void)
{
+ QCBORDecodeContext DCtx;
+ QCBORItem Item;
+ QCBORError uErr;
+
+ QCBORDecode_Init(&DCtx,
+ UsefulBuf_FROM_BYTE_ARRAY_LITERAL(spLargeArrayFake),
+ QCBOR_DECODE_MODE_NORMAL);
+ uErr = QCBORDecode_GetNext(&DCtx, &Item);
+ if(uErr != QCBOR_SUCCESS || Item.val.uCount != QCBOR_MAX_ITEMS_IN_ARRAY) {
+ return -100;
+ }
+
+ QCBORDecode_Init(&DCtx,
+ UsefulBuf_FROM_BYTE_ARRAY_LITERAL(spTooLargeArrayFake),
+ QCBOR_DECODE_MODE_NORMAL);
+ uErr = QCBORDecode_GetNext(&DCtx, &Item);
+ if(uErr != QCBOR_ERR_ARRAY_DECODE_TOO_LONG) {
+ return -101;
+ }
+
+ QCBORDecode_Init(&DCtx,
+ UsefulBuf_FROM_BYTE_ARRAY_LITERAL(spLargeMapFake),
+ QCBOR_DECODE_MODE_NORMAL);
+ uErr = QCBORDecode_GetNext(&DCtx, &Item);
+ if(uErr != QCBOR_SUCCESS || Item.val.uCount != QCBOR_MAX_ITEMS_IN_MAP) {
+ return -110;
+ }
+
+ QCBORDecode_Init(&DCtx,
+ UsefulBuf_FROM_BYTE_ARRAY_LITERAL(spTooLargeMapFake),
+ QCBOR_DECODE_MODE_NORMAL);
+ uErr = QCBORDecode_GetNext(&DCtx, &Item);
+ if(uErr != QCBOR_ERR_ARRAY_DECODE_TOO_LONG) {
+ return -111;
+ }
+
+
// Parse a moderatly complex map structure very thoroughly
int32_t nResult = ParseMapTest1(QCBOR_DECODE_MODE_NORMAL);
if(nResult) {
diff --git a/test/qcbor_encode_tests.c b/test/qcbor_encode_tests.c
index 44f97ab..1539023 100644
--- a/test/qcbor_encode_tests.c
+++ b/test/qcbor_encode_tests.c
@@ -959,10 +959,9 @@
static const uint8_t spExpectedEncodedSimpleIndefiniteLength[] = {
0x9f, 0xf5, 0xf4, 0xf6, 0xf7, 0xbf, 0x65, 0x55, 0x4e, 0x44, 0x65, 0x66, 0xf7, 0xff, 0xff};
-int32_t SimpleValuesIndefiniteLengthTest1(void)
+int32_t IndefiniteLengthTest(void)
{
QCBOREncodeContext ECtx;
- int nReturn = 0;
QCBOREncode_Init(&ECtx, UsefulBuf_FROM_BYTE_ARRAY(spBigBuf));
QCBOREncode_OpenArrayIndefiniteLength(&ECtx);
@@ -981,13 +980,38 @@
UsefulBufC ECBOR;
if(QCBOREncode_Finish(&ECtx, &ECBOR)) {
- nReturn = -1;
+ return -1;
}
- if(CheckResults(ECBOR, spExpectedEncodedSimpleIndefiniteLength))
+ if(CheckResults(ECBOR, spExpectedEncodedSimpleIndefiniteLength)) {
return -2;
+ }
- return(nReturn);
+
+#ifndef QCBOR_DISABLE_ENCODE_USAGE_GUARDS
+ QCBOREncode_Init(&ECtx, UsefulBuf_FROM_BYTE_ARRAY(spBigBuf));
+ QCBOREncode_OpenArrayIndefiniteLength(&ECtx);
+ QCBOREncode_CloseArray(&ECtx);
+ if(QCBOREncode_GetErrorState(&ECtx) != QCBOR_ERR_CLOSE_MISMATCH) {
+ return -3;
+ }
+
+ QCBOREncode_Init(&ECtx, UsefulBuf_FROM_BYTE_ARRAY(spBigBuf));
+ QCBOREncode_OpenArray(&ECtx);
+ QCBOREncode_CloseArrayIndefiniteLength(&ECtx);
+ if(QCBOREncode_GetErrorState(&ECtx) != QCBOR_ERR_CLOSE_MISMATCH) {
+ return -3;
+ }
+
+ QCBOREncode_Init(&ECtx, UsefulBuf_FROM_BYTE_ARRAY(spBigBuf));
+ QCBOREncode_OpenArrayIndefiniteLength(&ECtx);
+ QCBOREncode_CloseMapIndefiniteLength(&ECtx);
+ if(QCBOREncode_GetErrorState(&ECtx) != QCBOR_ERR_CLOSE_MISMATCH) {
+ return -3;
+ }
+#endif /* ! QCBOR_DISABLE_ENCODE_USAGE_GUARDS */
+
+ return 0;
}
#endif
@@ -1905,6 +1929,24 @@
return -11;
}
+#ifndef QCBOR_DISABLE_ENCODE_USAGE_GUARDS
+ // Seventh test, erroneous cancel
+ QCBOREncode_Init(&EC, UsefulBuf_FROM_BYTE_ARRAY(spBigBuf));
+ QCBOREncode_CancelBstrWrap(&EC);
+ uErr = QCBOREncode_GetErrorState(&EC);
+ if(uErr != QCBOR_ERR_TOO_MANY_CLOSES) {
+ return -12;
+ }
+
+ QCBOREncode_Init(&EC, UsefulBuf_FROM_BYTE_ARRAY(spBigBuf));
+ QCBOREncode_OpenArray(&EC);
+ QCBOREncode_CancelBstrWrap(&EC);
+ uErr = QCBOREncode_GetErrorState(&EC);
+ if(uErr != QCBOR_ERR_CLOSE_MISMATCH) {
+ return -13;
+ }
+#endif /* ! QCBOR_DISABLE_ENCODE_USAGE_GUARDS */
+
return 0;
}
@@ -2721,6 +2763,37 @@
return -130;
}
+
+#ifndef QCBOR_DISABLE_ENCODE_USAGE_GUARDS
+ /* ------ QCBOR_ERR_ARRAY_TOO_LONG -------- */
+ QCBOREncode_Init(&EC, Large);
+ QCBOREncode_OpenArray(&EC);
+ int i;
+ for(i = 0; i < QCBOR_MAX_ITEMS_IN_ARRAY; i++) {
+ QCBOREncode_AddInt64(&EC, 0);
+ }
+ if(QCBOREncode_GetErrorState(&EC)) {
+ return 250;
+ }
+ QCBOREncode_AddInt64(&EC, 0);
+ if(QCBOREncode_GetErrorState(&EC) != QCBOR_ERR_ARRAY_TOO_LONG) {
+ return 251;
+ }
+
+ QCBOREncode_Init(&EC, Large);
+ QCBOREncode_OpenMap(&EC);
+ for(i = 0; i < QCBOR_MAX_ITEMS_IN_MAP; i++) {
+ QCBOREncode_AddInt64ToMapN(&EC, 0,0);
+ }
+ if(QCBOREncode_GetErrorState(&EC)) {
+ return 250;
+ }
+ QCBOREncode_AddInt64ToMapN(&EC, 0,0);
+ if(QCBOREncode_GetErrorState(&EC) != QCBOR_ERR_ARRAY_TOO_LONG) {
+ return 251;
+ }
+#endif /* ! QCBOR_DISABLE_ENCODE_USAGE_GUARDS */
+
return 0;
}
@@ -2729,26 +2802,34 @@
/*
[
4([-1, 3]),
+ [-1, 4],
4([-20, 4759477275222530853136]),
+ [2, 4759477275222530853136],
4([9223372036854775807, -4759477275222530853137]),
5([300, 100]),
+ [600, 200],
5([-20, 4759477275222530853136]),
- 5([-9223372036854775808, -4759477275222530853137])
- ]
+ [4, 4759477275222530853136],
+ 5([-9223372036854775808, -4759477275222530853137])]
+ ]
*/
static const uint8_t spExpectedExponentAndMantissaArray[] = {
- 0x86, 0xC4, 0x82, 0x20, 0x03, 0xC4, 0x82, 0x33,
- 0xC2, 0x4A, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06,
- 0x07, 0x08, 0x09, 0x10, 0xC4, 0x82, 0x1B, 0x7F,
- 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xC3,
- 0x4A, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
- 0x08, 0x09, 0x10, 0xC5, 0x82, 0x19, 0x01, 0x2C,
- 0x18, 0x64, 0xC5, 0x82, 0x33, 0xC2, 0x4A, 0x01,
- 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09,
- 0x10, 0xC5, 0x82, 0x3B, 0x7F, 0xFF, 0xFF, 0xFF,
- 0xFF, 0xFF, 0xFF, 0xFF, 0xC3, 0x4A, 0x01, 0x02,
- 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x10};
-
+ 0x8A, 0xC4, 0x82, 0x20, 0x03, 0x82, 0x20, 0x04,
+ 0xC4, 0x82, 0x33, 0xC2, 0x4A, 0x01, 0x02, 0x03,
+ 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x10, 0x82,
+ 0x02, 0xC2, 0x4A, 0x01, 0x02, 0x03, 0x04, 0x05,
+ 0x06, 0x07, 0x08, 0x09, 0x10, 0xC4, 0x82, 0x1B,
+ 0x7F, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+ 0xC3, 0x4A, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06,
+ 0x07, 0x08, 0x09, 0x10, 0xC5, 0x82, 0x19, 0x01,
+ 0x2C, 0x18, 0x64, 0x82, 0x19, 0x02, 0x58, 0x18,
+ 0xC8, 0xC5, 0x82, 0x33, 0xC2, 0x4A, 0x01, 0x02,
+ 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x10,
+ 0x82, 0x04, 0xC2, 0x4A, 0x01, 0x02, 0x03, 0x04,
+ 0x05, 0x06, 0x07, 0x08, 0x09, 0x10, 0xC5, 0x82,
+ 0x3B, 0x7F, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+ 0xFF, 0xC3, 0x4A, 0x01, 0x02, 0x03, 0x04, 0x05,
+ 0x06, 0x07, 0x08, 0x09, 0x10};
/*
{
@@ -2827,10 +2908,14 @@
QCBOREncode_Init(&EC, UsefulBuf_FROM_BYTE_ARRAY(spBigBuf));
QCBOREncode_OpenArray(&EC);
QCBOREncode_AddDecimalFraction(&EC, 3, -1); // 3 * (10 ^ -1)
+ QCBOREncode_AddTDecimalFraction(&EC, QCBOR_ENCODE_AS_BORROWED, 4, -1); // 3 * (10 ^ -1)
QCBOREncode_AddDecimalFractionBigNum(&EC, BigNum , false, -20);
+ QCBOREncode_AddTDecimalFractionBigNum(&EC, QCBOR_ENCODE_AS_BORROWED, BigNum , false, 2);
QCBOREncode_AddDecimalFractionBigNum(&EC, BigNum, true, INT64_MAX);
QCBOREncode_AddBigFloat(&EC, 100, 300);
+ QCBOREncode_AddTBigFloat(&EC, QCBOR_ENCODE_AS_BORROWED, 200, 600);
QCBOREncode_AddBigFloatBigNum(&EC, BigNum, false, -20);
+ QCBOREncode_AddTBigFloatBigNum(&EC, QCBOR_ENCODE_AS_BORROWED, BigNum, false, 4);
QCBOREncode_AddBigFloatBigNum(&EC, BigNum, true, INT64_MIN);
QCBOREncode_CloseArray(&EC);
diff --git a/test/qcbor_encode_tests.h b/test/qcbor_encode_tests.h
index 43a6290..bc47c55 100644
--- a/test/qcbor_encode_tests.h
+++ b/test/qcbor_encode_tests.h
@@ -108,7 +108,7 @@
/*
Encodes basic maps and arrays with indefinite length
*/
-int32_t SimpleValuesIndefiniteLengthTest1(void);
+int32_t IndefiniteLengthTest(void);
/*
diff --git a/test/run_tests.c b/test/run_tests.c
index 8211a5a..cf806e9 100644
--- a/test/run_tests.c
+++ b/test/run_tests.c
@@ -146,7 +146,7 @@
TEST_ENTRY_DISABLED(TooLargeInputTest),
TEST_ENTRY(EncodeErrorTests),
#ifndef QCBOR_DISABLE_INDEFINITE_LENGTH_ARRAYS
- TEST_ENTRY(SimpleValuesIndefiniteLengthTest1),
+ TEST_ENTRY(IndefiniteLengthTest),
#endif
TEST_ENTRY(EncodeLengthThirtyoneTest),
TEST_ENTRY(CBORSequenceDecodeTests),