From b7a8b01471e2d97ec5de3c0cc4283373e4b36c3e Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Thu, 9 Jan 2025 21:45:10 +0100 Subject: [PATCH] sys/net/gcoap: get rid of API abuse Calling `coap_get_token()` and `coap_get_token_length()` on an (mostly) uninitialized `coap_pkt_t` did work so far due to implementation details matching the expectations, but this is not backed up by any API contract. This fixes the API abuse by introducing and using a new API that does read a token and token length from a CoAP over UDP packet out of a buffer. This now provides the behavior expected by the caller and commits to it via API contract. Co-authored-by: mguetschow --- sys/include/net/nanocoap.h | 48 +++++++++++++++++++++++++ sys/net/application_layer/gcoap/gcoap.c | 13 +++---- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/sys/include/net/nanocoap.h b/sys/include/net/nanocoap.h index b77b557b5bad..3efb984f23e1 100644 --- a/sys/include/net/nanocoap.h +++ b/sys/include/net/nanocoap.h @@ -567,6 +567,54 @@ static inline void *coap_get_token(const coap_pkt_t *pkt) return coap_hdr_data_ptr(pkt->hdr); } +/** + * @brief Get the token from a CoAP over UDP packet from a buffer + * + * @warning This function will happily read past the buffer on invalid input. + * Avoid using this function. Use @ref coap_parse instead. + * + * @pre This assumes that the buffer has already been validated (e.g. using + * @ref coap_udp_parse). + */ +static inline void *coap_get_token_from_udp_buf(uint8_t *buf) +{ + coap_udp_hdr_t *hdr = (void *)buf; + + uint8_t tkl = hdr->ver_t_tkl & 0xf; + size_t tkl_ext_len = coap_get_ext_tkl_field_len_from_tkl_val(tkl); + return buf + sizeof(coap_udp_hdr_t) + tkl_ext_len; +} + +/** + * @brief Get the token length from a CoAP over UDP packet from a buffer + * + * @warning This function will happily read past the buffer on invalid input. + * Avoid using this function. Use @ref coap_parse instead. + * + * @pre This assumes that the buffer has already been validated (e.g. using + * @ref coap_udp_parse). + */ +static inline size_t coap_get_token_len_from_udp_buf(const uint8_t *buf) +{ + coap_udp_hdr_t *hdr = (void *)buf; + uint8_t tkl = hdr->ver_t_tkl & 0xf; + size_t tkl_ext_len = coap_get_ext_tkl_field_len_from_tkl_val(tkl); + + if (tkl_ext_len == 0) { + return tkl; + } + + if (tkl_ext_len == 1) { + /* In https://www.rfc-editor.org/rfc/rfc8974#section-2.1 the magic + * number 13 has no name, so we don't make up one here */ + return buf[sizeof(coap_udp_hdr_t)] + 13; + } + + /* In https://www.rfc-editor.org/rfc/rfc8974#section-2.1 the magic + * number 269 has no name, so we don't make up one here */ + return byteorder_bebuftohs(buf + sizeof(coap_udp_hdr_t)) + 269; +} + /** * @brief Get the total length of a CoAP packet in the packet buffer * diff --git a/sys/net/application_layer/gcoap/gcoap.c b/sys/net/application_layer/gcoap/gcoap.c index 6f36a13594e2..7e3701471662 100644 --- a/sys/net/application_layer/gcoap/gcoap.c +++ b/sys/net/application_layer/gcoap/gcoap.c @@ -945,17 +945,13 @@ static int _find_resource(gcoap_socket_type_t tl_type, static gcoap_request_memo_t* _find_req_memo_by_token(const sock_udp_ep_t *remote, const uint8_t *token, size_t tkl) { - /* no need to initialize struct; we only care about buffer contents below */ - coap_pkt_t memo_pdu_data; - coap_pkt_t *memo_pdu = &memo_pdu_data; - for (int i = 0; i < CONFIG_GCOAP_REQ_WAITING_MAX; i++) { if (_coap_state.open_reqs[i].state == GCOAP_MEMO_UNUSED) { continue; } gcoap_request_memo_t *memo = &_coap_state.open_reqs[i]; - memo_pdu->hdr = gcoap_request_memo_get_hdr(memo); + uint8_t *buf = (uint8_t *)gcoap_request_memo_get_hdr(memo); /* verbose debug to catch bugs with request/response matching */ #if SOCK_HAS_IPV4 @@ -972,11 +968,12 @@ static gcoap_request_memo_t* _find_req_memo_by_token(const sock_udp_ep_t *remote tkl); #endif - if (coap_get_token_len(memo_pdu) != tkl) { - DEBUG("Token length mismatch %u\n", coap_get_token_len(memo_pdu)); + size_t memo_tkl = coap_get_token_len_from_udp_buf(buf); + if (memo_tkl != tkl) { + DEBUG("Token length mismatch %" PRIuSIZE "\n", memo_tkl); continue; } - const uint8_t *memo_token = coap_get_token(memo_pdu); + const uint8_t *memo_token = coap_get_token_from_udp_buf(buf); if (memcmp(token, memo_token, tkl)) { DEBUG("Token mismatch 0x%02x%02x%02x%02x%02x%02x%02x%02x\n", memo_token[0], memo_token[1], memo_token[2], memo_token[3],