helper: fix out of bound access on unmarshal (#17)

libmcu · Aug 7, 2023 · f3d1696 · f3d1696
1 parent 30bbd24
commit f3d1696
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 3 deletions.
diff --git a/include/cbor/helper.h b/include/cbor/helper.h
@@ -16,6 +16,7 @@ extern "C" {
 
 struct cbor_parser {
 	const void *key;
+	size_t keylen;
 	void (*run)(const cbor_reader_t *reader,
 			const struct cbor_parser *parser,
 			const cbor_item_t *item, void *arg);

diff --git a/src/helper.c b/src/helper.c
@@ -26,7 +26,7 @@ static const struct cbor_parser *get_parser(const struct parser_ctx *ctx,
 			continue;
 		}
 
-		if (strkey && strkey_len) {
+		if (strkey && strkey_len && p->keylen >= strkey_len) {
 			if (memcmp(p->key, strkey, strkey_len) == 0) {
 				return p;
 			}

diff --git a/tests/src/helper_test.cpp b/tests/src/helper_test.cpp
@@ -22,9 +22,25 @@ static void parse_key(const cbor_reader_t *reader,
 	mock().actualCall(__func__);
 }
 
+static void parse_strkey(const cbor_reader_t *reader,
+		const struct cbor_parser *parser,
+		const cbor_item_t *item, void *arg) {
+	mock().actualCall(__func__);
+}
+
+static void parse_intkey(const cbor_reader_t *reader,
+		const struct cbor_parser *parser,
+		const cbor_item_t *item, void *arg) {
+	mock().actualCall(__func__);
+}
+
 static const struct cbor_parser parsers[] = {
-	{ .key = "certificate", .run = parse_cert },
-	{ .key = "privateKey",  .run = parse_key },
+	{ .key = "certificate", .keylen = 11, .run = parse_cert },
+	{ .key = "privateKey",  .keylen = 10, .run = parse_key},
+	{ .key = "strkey1",     .keylen = 7,  .run = parse_strkey },
+	{ .key = (const void *)1,             .run = parse_intkey },
+	{ .key = "strkey2",     .keylen = 7,  .run = parse_strkey },
+	{ .key = (const void *)2,             .run = parse_intkey },
 };
 
 TEST_GROUP(Helper) {
@@ -74,3 +90,19 @@ TEST(Helper, unmarshal_ShouldReturnFalse_WhenInvalidMessageGiven) {
 			sizeof(parsers) / sizeof(*parsers),
 			msg, sizeof(msg), 0));
 }
+
+TEST(Helper, ShouldUnmarshal_WhenBothOfStrKeyAndIntKeyGiven) {
+	uint8_t msg[] = {
+		0xA4, 0x01, 0x66, 0x69, 0x6E, 0x74, 0x6B, 0x65, 
+		0x79, 0x67, 0x73, 0x74, 0x72, 0x6B, 0x65, 0x79, 
+		0x32, 0x63, 0x61, 0x62, 0x63, 0x02, 0x01, 0x67, 
+		0x73, 0x74, 0x72, 0x6B, 0x65, 0x79, 0x31, 0x02, 
+	};
+
+	mock().expectNCalls(2, "parse_intkey");
+	mock().expectNCalls(2, "parse_strkey");
+
+	LONGS_EQUAL(true, cbor_unmarshal(&reader, parsers,
+			sizeof(parsers) / sizeof(*parsers),
+			msg, sizeof(msg), 0));
+}