Important spiffy decode fix to correctly handle some bad input cases (#150)
When fetching map items by label or using QCBORDecode_VGetNextConsume some bad input would result in an infinite loop. The bad input includes labels that are not strings or integers, non-well-formed maps and arrays and invalid tag content for tags like string and epoch dates. This can result in a denial-of-service attack, but not an overrun security issue.
These errors are now considered unrecoverable:
QCBOR_ERR_INDEF_LEN_ARRAYS_DISABLED
QCBOR_ERR_INDEF_LEN_STRINGS_DISABLED
QCBOR_ERR_MAP_LABEL_TYPE
QCBOR_ERR_UNRECOVERABLE_TAG_CONTENT
The error codes have been renumbered.
Spiffy decode traversal of maps and arrays with these errors will now correctly and directly fail. Without the fix they might fail incorrectly or get stuck in an infinite loop.
Tests to cover these cases are added.
Thanks to Jan Brandstetter and a fuzzing project for finding this.
* Fix error classification to make Consume work correctly
* Bring tests cases up to date with new behavior
* Documentation fixes
* error code renumbering
* Minor clean up
* fix one more test case
Co-authored-by: Laurence Lundblade <lgl@securitytheory.com>
diff --git a/test/qcbor_decode_tests.c b/test/qcbor_decode_tests.c
index 755d032..431836c 100644
--- a/test/qcbor_decode_tests.c
+++ b/test/qcbor_decode_tests.c
@@ -2561,7 +2561,7 @@
Untagged values
*/
static const uint8_t spSpiffyDateTestInput[] = {
- 0x86,
+ 0x86, // array of 6 items
0xc1,
0xfb, 0xc3, 0xdf, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, // -9.2233720368547748E+18, too negative
@@ -2572,12 +2572,6 @@
0xc1, // tag for epoch date
0xf9, 0xfc, 0x00, // Half-precision -Infinity
- 0xc1, // tag for epoch date
- 0x80, // Erroneous empty array as content for date
-
- 0xc0, // tag for string date
- 0xa0, // Erroneous empty map as content for date
-
0xad, // Open a map for tests involving labels.
0x00,
@@ -2634,7 +2628,16 @@
/* Untagged days-count epoch date */
0x11,
- 0x19, 0x0F, 0x9A /* 3994 */
+ 0x19, 0x0F, 0x9A, /* 3994 */
+
+ // End of map, back to array
+
+ // These two at the end because they are unrecoverable errors
+ 0xc1, // tag for epoch date
+ 0x80, // Erroneous empty array as content for date
+
+ 0xc0, // tag for string date
+ 0xa0 // Erroneous empty map as content for date
};
@@ -2674,19 +2677,7 @@
return 2;
}
- // Bad content for epoch date
- QCBORDecode_GetEpochDate(&DC, QCBOR_TAG_REQUIREMENT_TAG, &nEpochDateFail);
- uError = QCBORDecode_GetAndResetError(&DC);
- if(uError != QCBOR_ERR_BAD_OPT_TAG) {
- return 3;
- }
- // Bad content for string date
- QCBORDecode_GetDateString(&DC, QCBOR_TAG_REQUIREMENT_TAG, &StringDate1);
- uError = QCBORDecode_GetAndResetError(&DC);
- if(uError != QCBOR_ERR_BAD_OPT_TAG) {
- return 4;
- }
QCBORDecode_EnterMap(&DC, NULL);
@@ -2826,9 +2817,27 @@
&nEpochDays2);
QCBORDecode_ExitMap(&DC);
+ if(QCBORDecode_GetError(&DC) != QCBOR_SUCCESS) {
+ return 3001;
+ }
+
+ // Bad content for epoch date
+ QCBORDecode_GetEpochDate(&DC, QCBOR_TAG_REQUIREMENT_TAG, &nEpochDateFail);
+ uError = QCBORDecode_GetAndResetError(&DC);
+ if(uError != QCBOR_ERR_UNRECOVERABLE_TAG_CONTENT) {
+ return 3;
+ }
+
+ // Bad content for string date
+ QCBORDecode_GetDateString(&DC, QCBOR_TAG_REQUIREMENT_TAG, &StringDate1);
+ uError = QCBORDecode_GetAndResetError(&DC);
+ if(uError != QCBOR_ERR_UNRECOVERABLE_TAG_CONTENT) {
+ return 4;
+ }
+
QCBORDecode_ExitArray(&DC);
uError = QCBORDecode_Finish(&DC);
- if(uError) {
+ if(uError != QCBOR_ERR_UNRECOVERABLE_TAG_CONTENT) {
return 1000 + (int32_t)uError;
}
@@ -3031,6 +3040,10 @@
0xc0, // tag for string date
0x4a, '1','9','8','5','-','0','4','-','1','2', // Date string in byte string
+
+ // This last case makes the array untraversable because it is
+ // an uncrecoverable error. Make sure it stays last and is the only
+ // instance so the other tests can work.
};
@@ -3432,11 +3445,15 @@
}
// tagged date string with a byte string
QCBORDecode_GetDateString(&DCtx, QCBOR_TAG_REQUIREMENT_TAG, &DateString);
- if(QCBORDecode_GetAndResetError(&DCtx) != QCBOR_ERR_BAD_OPT_TAG) {
+ if(QCBORDecode_GetAndResetError(&DCtx) != QCBOR_ERR_UNRECOVERABLE_TAG_CONTENT) {
return 103;
}
+ // The exit errors out because the last item, the date string with
+ // bad content makes the array untraversable (the bad date string
+ // could have tag content of an array or such that is not consumed
+ // by the date decoding).
QCBORDecode_ExitArray(&DCtx);
- if(QCBORDecode_Finish(&DCtx) != QCBOR_SUCCESS) {
+ if(QCBORDecode_Finish(&DCtx) != QCBOR_ERR_UNRECOVERABLE_TAG_CONTENT) {
return 104;
}
@@ -3465,11 +3482,12 @@
}
// tagged date string with a byte string
QCBORDecode_GetDateString(&DCtx, QCBOR_TAG_REQUIREMENT_OPTIONAL_TAG, &DateString);
- if(QCBORDecode_GetAndResetError(&DCtx) != QCBOR_ERR_BAD_OPT_TAG) {
+ if(QCBORDecode_GetAndResetError(&DCtx) != QCBOR_ERR_UNRECOVERABLE_TAG_CONTENT) {
return 203;
}
+ // See comments above
QCBORDecode_ExitArray(&DCtx);
- if(QCBORDecode_Finish(&DCtx) != QCBOR_SUCCESS) {
+ if(QCBORDecode_Finish(&DCtx) != QCBOR_ERR_UNRECOVERABLE_TAG_CONTENT) {
return 204;
}
@@ -3500,11 +3518,12 @@
}
// tagged date string with a byte string
QCBORDecode_GetDateString(&DCtx, QCBOR_TAG_REQUIREMENT_NOT_A_TAG, &DateString);
- if(QCBORDecode_GetAndResetError(&DCtx) != QCBOR_ERR_BAD_OPT_TAG) {
+ if(QCBORDecode_GetAndResetError(&DCtx) != QCBOR_ERR_UNRECOVERABLE_TAG_CONTENT) {
return 304;
}
+ // See comments above
QCBORDecode_ExitArray(&DCtx);
- if(QCBORDecode_Finish(&DCtx) != QCBOR_SUCCESS) {
+ if(QCBORDecode_Finish(&DCtx) != QCBOR_ERR_UNRECOVERABLE_TAG_CONTENT) {
return 305;
}
@@ -5179,23 +5198,21 @@
/*
Too many tags
- Invalid tag content
Duplicate label
Integer overflow
Date overflow
{
1: 224(225(226(227(4(0))))),
- 2: 1(h''),
3: -18446744073709551616,
4: 1(1.0e+300),
- 5: 0, 8: 8
+ 5: 0,
+ 8: 8
}
*/
static const uint8_t spRecoverableMapErrors[] = {
- 0xa7,
+ 0xa6,
0x01, 0xd8, 0xe0, 0xd8, 0xe1, 0xd8, 0xe2, 0xd8, 0xe3, 0xd8, 0x04, 0x00,
- 0x02, 0xc1, 0x40,
0x03, 0x3b, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0x04, 0xc1, 0xfb, 0x7e, 0x37, 0xe4, 0x3c, 0x88, 0x00, 0x75, 0x9c,
0x05, 0x00,
@@ -5234,6 +5251,37 @@
0xa1, 0x14, 0x1f,
};
+
+/* Array of length 3, but only two items. */
+const unsigned char spBadConsumeInput[] = {
+ 0x83, 0x00, 0x00
+};
+
+/* Tag nesting too deep. */
+const unsigned char spBadConsumeInput2[] = {
+ 0x81,
+ 0xD8, 0x37,
+ 0xD8, 0x2C,
+ 0xD8, 0x21,
+ 0xD6,
+ 0xCB,
+ 00
+};
+
+const unsigned char spBadConsumeInput3[] = {
+ 0x81, 0xc0, 0x81, 0x00
+};
+
+const unsigned char spBadConsumeInput4[] = {
+ 0x81, 0x9f, 0x00, 0xff
+};
+
+const unsigned char spBadConsumeInput5[] = {
+ 0xa1, 0x80, 0x00
+};
+
+
+
int32_t EnterMapTest()
{
QCBORItem Item1;
@@ -5450,12 +5498,6 @@
(void)QCBORDecode_GetAndResetError(&DCtx);
- QCBORDecode_GetEpochDateInMapN(&DCtx, 0x02, QCBOR_TAG_REQUIREMENT_TAG, &nInt);
- uErr = QCBORDecode_GetAndResetError(&DCtx);
- if(uErr != QCBOR_ERR_BAD_OPT_TAG) {
- return 2022;
- }
-
QCBORDecode_GetInt64InMapN(&DCtx, 0x03, &nInt);
uErr = QCBORDecode_GetAndResetError(&DCtx);
if(uErr != QCBOR_ERR_INT_OVERFLOW) {
@@ -5575,6 +5617,42 @@
return 2500;
}
+ QCBORDecode_Init(&DCtx, UsefulBuf_FROM_BYTE_ARRAY_LITERAL(spBadConsumeInput), 0);
+ QCBORDecode_VGetNextConsume(&DCtx, &Item1);
+ if(QCBORDecode_GetError(&DCtx) != QCBOR_ERR_NO_MORE_ITEMS) {
+ return 2600;
+ }
+
+ QCBORDecode_Init(&DCtx, UsefulBuf_FROM_BYTE_ARRAY_LITERAL(spBadConsumeInput2), 0);
+ QCBORDecode_VGetNextConsume(&DCtx, &Item1);
+ if(QCBORDecode_GetError(&DCtx) != QCBOR_SUCCESS) {
+ return 2700;
+ }
+
+ QCBORDecode_Init(&DCtx, UsefulBuf_FROM_BYTE_ARRAY_LITERAL(spBadConsumeInput3), 0);
+ QCBORDecode_VGetNextConsume(&DCtx, &Item1);
+ if(QCBORDecode_GetError(&DCtx) != QCBOR_ERR_UNRECOVERABLE_TAG_CONTENT) {
+ return 2800;
+ }
+
+ QCBORDecode_Init(&DCtx, UsefulBuf_FROM_BYTE_ARRAY_LITERAL(spBadConsumeInput4), 0);
+ QCBORDecode_VGetNextConsume(&DCtx, &Item1);
+#ifndef QCBOR_DISABLE_INDEFINITE_LENGTH_ARRAYS
+ if(QCBORDecode_GetError(&DCtx) != QCBOR_SUCCESS) {
+ return 2900;
+ }
+#else
+ if(QCBORDecode_GetError(&DCtx) != QCBOR_ERR_INDEF_LEN_ARRAYS_DISABLED) {
+ return 2901;
+ }
+#endif
+
+ QCBORDecode_Init(&DCtx, UsefulBuf_FROM_BYTE_ARRAY_LITERAL(spBadConsumeInput5), 0);
+ QCBORDecode_VGetNextConsume(&DCtx, &Item1);
+ if(QCBORDecode_GetError(&DCtx) != QCBOR_ERR_MAP_LABEL_TYPE) {
+ return 3000;
+ }
+
return nReturn;
}