From edf016a6cb79baac7aef5c2e8cdc5550ca8d9bcf Mon Sep 17 00:00:00 2001 From: Maciej Jurczak Date: Thu, 21 May 2020 16:30:15 +0200 Subject: [PATCH] nanocoap: Added token length validation. Implemented a check in coap_parse() to verify if TKL value is within valid range as specified by RFC7252. The token length must be within 0-8 range, any other value should be considered as invalid and the packet should produce message format error. A test case was added to tests-nanocoap.c to verify correct behavior in case of TKL in range and out of range. Update sys/net/application_layer/nanocoap/nanocoap.c Prefixed debug message with module name and abbreviations expanded. Co-authored-by: Martine Lenders Update sys/net/application_layer/nanocoap/nanocoap.c Prefixed debug message with module name and abbreviations expanded. Co-authored-by: Martine Lenders --- sys/include/net/coap.h | 7 ++++ sys/net/application_layer/nanocoap/nanocoap.c | 10 ++++-- .../unittests/tests-nanocoap/tests-nanocoap.c | 35 +++++++++++++++++++ 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/sys/include/net/coap.h b/sys/include/net/coap.h index 290cfb6bc9..61f22b4aad 100644 --- a/sys/include/net/coap.h +++ b/sys/include/net/coap.h @@ -161,6 +161,13 @@ extern "C" { #define COAP_OBS_DEREGISTER (1) /** @} */ +/** + * @name CoAP message format constants + * @{ + */ +#define COAP_TOKEN_LENGTH_MAX (8) +/** @} */ + /** * @defgroup net_coap_conf CoAP compile configurations * @ingroup net_coap diff --git a/sys/net/application_layer/nanocoap/nanocoap.c b/sys/net/application_layer/nanocoap/nanocoap.c index c403fd16ac..f375666e3a 100644 --- a/sys/net/application_layer/nanocoap/nanocoap.c +++ b/sys/net/application_layer/nanocoap/nanocoap.c @@ -79,13 +79,17 @@ int coap_parse(coap_pkt_t *pkt, uint8_t *buf, size_t len) } /* token value (tkl bytes) */ - if (coap_get_token_len(pkt)) { + if (coap_get_token_len(pkt) == 0) { + pkt->token = NULL; + } + else if (coap_get_token_len(pkt) <= COAP_TOKEN_LENGTH_MAX) { pkt->token = pkt_pos; /* pkt_pos range is validated after options parsing loop below */ pkt_pos += coap_get_token_len(pkt); } else { - pkt->token = NULL; + DEBUG("nanocoap: token length invalid\n"); + return -EBADMSG; } coap_optpos_t *optpos = pkt->options; @@ -134,7 +138,7 @@ int coap_parse(coap_pkt_t *pkt, uint8_t *buf, size_t len) } if (pkt_pos > pkt_end) { - DEBUG("bad pkt len\n"); + DEBUG("nanocoap: bad packet length\n"); return -EBADMSG; } diff --git a/tests/unittests/tests-nanocoap/tests-nanocoap.c b/tests/unittests/tests-nanocoap/tests-nanocoap.c index 48be74a772..00d5c85147 100644 --- a/tests/unittests/tests-nanocoap/tests-nanocoap.c +++ b/tests/unittests/tests-nanocoap/tests-nanocoap.c @@ -802,6 +802,40 @@ static void test_nanocoap__add_get_proxy_uri(void) TEST_ASSERT_EQUAL_INT(0, strncmp((char *) proxy_uri, (char *) uri, len)); } +/* + * Verifies that coap_parse() recognizes token length bigger than allowed. + */ +static void test_nanocoap__token_length_over_limit(void) +{ + /* RFC7252 states that TKL must be within 0-8: + * "Lengths 9-15 are reserved, MUST NOT be sent, + * and MUST be processed as a message format error." + */ + uint16_t msgid = 0xABCD; + uint8_t buf_invalid[] = { + 0x49, 0x01, 0xAB, 0xCD, + 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99 + }; + uint8_t buf_valid[] = { + 0x48, 0x01, 0xAB, 0xCD, + 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88 + }; + coap_pkt_t pkt; + + /* Valid packet (TKL = 8) */ + int res = coap_parse(&pkt, buf_valid, sizeof(buf_valid)); + + TEST_ASSERT_EQUAL_INT(0, res); + TEST_ASSERT_EQUAL_INT(1, coap_get_code_raw(&pkt)); + TEST_ASSERT_EQUAL_INT(msgid, coap_get_id(&pkt)); + TEST_ASSERT_EQUAL_INT(8, coap_get_token_len(&pkt)); + TEST_ASSERT_EQUAL_INT(0, pkt.payload_len); + + /* Invalid packet (TKL = 9) */ + res = coap_parse(&pkt, buf_invalid, sizeof(buf_invalid)); + TEST_ASSERT_EQUAL_INT(-EBADMSG, res); +} + Test *tests_nanocoap_tests(void) { EMB_UNIT_TESTFIXTURES(fixtures) { @@ -828,6 +862,7 @@ Test *tests_nanocoap_tests(void) new_TestFixture(test_nanocoap__empty), new_TestFixture(test_nanocoap__add_path_unterminated_string), new_TestFixture(test_nanocoap__add_get_proxy_uri), + new_TestFixture(test_nanocoap__token_length_over_limit), }; EMB_UNIT_TESTCALLER(nanocoap_tests, NULL, NULL, fixtures);