Parser: validate that maps have both key and value items
If a map end (Break byte) occurs before we've read the concrete item for
the value, then the map is invalid.
Fixes #167
Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
diff --git a/src/cbor.h b/src/cbor.h
index fd145be..d6360d8 100644
--- a/src/cbor.h
+++ b/src/cbor.h
@@ -1,6 +1,6 @@
/****************************************************************************
**
-** Copyright (C) 2017 Intel Corporation
+** Copyright (C) 2019 Intel Corporation
**
** Permission is hereby granted, free of charge, to any person obtaining a copy
** of this software and associated documentation files (the "Software"), to deal
@@ -270,7 +270,8 @@
CborIteratorFlag_NegativeInteger = 0x02,
CborIteratorFlag_IteratingStringChunks = 0x02,
CborIteratorFlag_UnknownLength = 0x04,
- CborIteratorFlag_ContainerIsMap = 0x20
+ CborIteratorFlag_ContainerIsMap = 0x20,
+ CborIteratorFlag_NextIsMapKey = 0x40
};
struct CborParser
diff --git a/src/cborparser.c b/src/cborparser.c
index 45de222..35cd78d 100644
--- a/src/cborparser.c
+++ b/src/cborparser.c
@@ -214,6 +214,10 @@
static CborError preparse_value(CborValue *it)
{
+ enum {
+ /* flags to keep */
+ FlagsToKeep = CborIteratorFlag_ContainerIsMap | CborIteratorFlag_NextIsMapKey
+ };
const CborParser *parser = it->parser;
it->type = CborInvalidType;
@@ -224,7 +228,7 @@
uint8_t descriptor = *it->ptr;
uint8_t type = descriptor & MajorTypeMask;
it->type = type;
- it->flags = 0;
+ it->flags &= FlagsToKeep;
it->extra = (descriptor &= SmallValueMask);
if (descriptor > Value64Bit) {
@@ -302,6 +306,11 @@
{
if (it->remaining == UINT32_MAX && it->ptr != it->parser->end && *it->ptr == (uint8_t)BreakByte) {
/* end of map or array */
+ if ((it->flags & CborIteratorFlag_ContainerIsMap && it->flags & CborIteratorFlag_NextIsMapKey)
+ || it->type == CborTagType) {
+ /* but we weren't expecting it! */
+ return CborErrorUnexpectedBreak;
+ }
++it->ptr;
it->type = CborInvalidType;
it->remaining = 0;
@@ -313,13 +322,20 @@
static CborError preparse_next_value(CborValue *it)
{
+ /* tags don't count towards item totals or whether we've successfully
+ * read a map's key or value */
+ bool itemCounts = it->type != CborTagType;
+
if (it->remaining != UINT32_MAX) {
- /* don't decrement the item count if the current item is tag: they don't count */
- if (it->type != CborTagType && --it->remaining == 0) {
+ if (itemCounts && --it->remaining == 0) {
it->type = CborInvalidType;
return CborNoError;
}
}
+ if (itemCounts) {
+ /* toggle the flag indicating whether this was a map key */
+ it->flags ^= CborIteratorFlag_NextIsMapKey;
+ }
return preparse_next_value_nodecrement(it);
}
@@ -381,6 +397,7 @@
it->parser = parser;
it->ptr = buffer;
it->remaining = 1; /* there's one type altogether, usually an array or map */
+ it->flags = 0;
return preparse_value(it);
}
@@ -586,6 +603,7 @@
*/
CborError cbor_value_enter_container(const CborValue *it, CborValue *recursed)
{
+ cbor_static_assert(CborIteratorFlag_ContainerIsMap == (CborMapType & ~CborArrayType));
cbor_assert(cbor_value_is_container(it));
*recursed = *it;
@@ -618,6 +636,7 @@
return CborNoError;
}
}
+ recursed->flags = (recursed->type & CborIteratorFlag_ContainerIsMap);
return preparse_next_value_nodecrement(recursed);
}
diff --git a/tests/parser/tst_parser.cpp b/tests/parser/tst_parser.cpp
index 97e9ed4..28967cc 100644
--- a/tests/parser/tst_parser.cpp
+++ b/tests/parser/tst_parser.cpp
@@ -1563,6 +1563,10 @@
QTest::newRow("array-no-break2") << raw("\x81\x9f\0") << 0 << CborErrorUnexpectedEOF;
QTest::newRow("map-no-break1") << raw("\x81\xbf") << 0 << CborErrorUnexpectedEOF;
QTest::newRow("map-no-break2") << raw("\x81\xbf\0\0") << 0 << CborErrorUnexpectedEOF;
+ QTest::newRow("map-break-after-key") << raw("\x81\xbf\0\xff") << 0 << CborErrorUnexpectedBreak;
+ QTest::newRow("map-break-after-second-key") << raw("\x81\xbf\x64xyzw\x04\x00\xff") << 0 << CborErrorUnexpectedBreak;
+ QTest::newRow("map-break-after-value-tag") << raw("\x81\xbf\0\xc0\xff") << 0 << CborErrorUnexpectedBreak;
+ QTest::newRow("map-break-after-value-tag2") << raw("\x81\xbf\0\xd8\x20\xff") << 0 << CborErrorUnexpectedBreak;
// check for pointer additions wrapping over the limit of the address space
CborError tooLargeOn32bit = (sizeof(void *) == 4) ? CborErrorDataTooLarge : CborErrorUnexpectedEOF;
@@ -1674,6 +1678,24 @@
QCOMPARE(err2, expectedError);
QCOMPARE(err3, expectedError);
}
+
+ // see if we've got a map
+ if (QByteArray(QTest::currentDataTag()).startsWith("map")) {
+ w.init(data, uint32_t(flags)); // reinit
+ QVERIFY(cbor_value_is_array(&w.first));
+
+ CborValue map;
+ CborError err = cbor_value_enter_container(&w.first, &map);
+ if (err == CborNoError) {
+ QVERIFY(cbor_value_is_map(&map));
+ CborValue element;
+ err = cbor_value_map_find_value(&map, "foobar", &element);
+ if (err == CborNoError)
+ QVERIFY(!cbor_value_is_valid(&element));
+ }
+
+ QCOMPARE(err, expectedError);
+ }
}
void tst_Parser::strictValidation_data()
