From 07c880b56edc5eba77f386bebddee02fad2ab2ea Mon Sep 17 00:00:00 2001 From: Jon Shallow Date: Thu, 15 May 2025 09:54:09 +0100 Subject: [PATCH] coap_pdu.c: Report options with invalid length Malformed PDUs were responsed to with a RST. Now request PDUs with invalid lengths are also reported with a 4.02 response. See RFC7252 5.4.3 and 5.4.1. --- include/coap3/coap_pdu.h | 1 + include/coap3/coap_pdu_internal.h | 24 +++++++++++++- src/coap_io_lwip.c | 55 ++++++++++++++++++++++++++++--- src/coap_net.c | 35 ++++++++++++++++++-- src/coap_pdu.c | 52 ++++++++++++++++++++++------- 5 files changed, 147 insertions(+), 20 deletions(-) diff --git a/include/coap3/coap_pdu.h b/include/coap3/coap_pdu.h index 804ebb6d..3e073b4a 100644 --- a/include/coap3/coap_pdu.h +++ b/include/coap3/coap_pdu.h @@ -134,6 +134,7 @@ typedef enum coap_request_t { #define COAP_OPTION_ACCEPT 17 /* C___E__, uint, 0-2 B, RFC7252 */ #define COAP_OPTION_Q_BLOCK1 19 /* CU__E_U, uint, 0-3 B, RFC9177 */ #define COAP_OPTION_LOCATION_QUERY 20 /* ___RE__, String, 0-255 B, RFC7252 */ +#define COAP_OPTION_EDHOC 21 /* C_____U, empty, 0 B, RFC9668 */ #define COAP_OPTION_BLOCK2 23 /* CU-_E_U, uint, 0-3 B, RFC7959 */ #define COAP_OPTION_BLOCK1 27 /* CU-_E_U, uint, 0-3 B, RFC7959 */ #define COAP_OPTION_SIZE2 28 /* __N_E_U, uint, 0-4 B, RFC7959 */ diff --git a/include/coap3/coap_pdu_internal.h b/include/coap3/coap_pdu_internal.h index 26cdb31c..9b359129 100644 --- a/include/coap3/coap_pdu_internal.h +++ b/include/coap3/coap_pdu_internal.h @@ -177,6 +177,27 @@ struct coap_pdu_t { coap_binary_t *data_free; /**< Data to be freed off by coap_delete_pdu() */ }; +/** + * Parses @p data into the CoAP PDU structure given in @p result. + * The target pdu must be large enough to hold the token, options and data. + * This function returns @c 0 on error or a number greater than zero on success. + * + * @param proto Session's protocol + * @param data The raw data to parse as CoAP PDU. + * @param length The actual size of @p data. + * @param pdu The PDU structure to fill. Note that the structure must + * provide space to hold the token, optional options and + * optional data. + * @param error_opts Filled in with any options that have an error in the parsing. + * + * @return 1 on success or @c 0 on error. + */ +int coap_pdu_parse2(coap_proto_t proto, + const uint8_t *data, + size_t length, + coap_pdu_t *pdu, + coap_opt_filter_t *error_opts); + /** * Dynamically grows the size of @p pdu to @p new_size. The new size * must not exceed the PDU's configure maximum size. On success, this @@ -243,10 +264,11 @@ int coap_pdu_parse_header(coap_pdu_t *pdu, coap_proto_t proto); * marker. * * @param pdu The PDU structure to check. + * @param error_opts Updated with error option(s) on failure. * * @return 1 on success or @c 0 on error. */ -int coap_pdu_parse_opt(coap_pdu_t *pdu); +int coap_pdu_parse_opt(coap_pdu_t *pdu, coap_opt_filter_t *error_opts); /** * Clears any contents from @p pdu and resets @c used_size, diff --git a/src/coap_io_lwip.c b/src/coap_io_lwip.c index 768f3a53..9b76ff2e 100644 --- a/src/coap_io_lwip.c +++ b/src/coap_io_lwip.c @@ -195,12 +195,34 @@ coap_recvc(void *arg, struct udp_pcb *upcb, struct pbuf *p, } pbuf_free(p); } else { + coap_opt_filter_t error_opts; + pdu = coap_pdu_from_pbuf(p); if (!pdu) goto error; - if (!coap_pdu_parse(session->proto, p->payload, p->len, pdu)) { - goto error; + coap_option_filter_clear(&error_opts); + if (!coap_pdu_parse2(session->proto, p->payload, p->len, pdu, &error_opts)) { + coap_handle_event_lkd(session->context, COAP_EVENT_BAD_PACKET, session); + coap_log_warn("discard malformed PDU\n"); + if (error_opts.mask && COAP_PDU_IS_REQUEST(pdu)) { + coap_pdu_t *response = + coap_new_error_response(pdu, + COAP_RESPONSE_CODE(402), &error_opts); + if (!response) { + coap_log_warn("coap_handle_dgram: cannot create error response\n"); + } else { + if (coap_send_internal(session, response, NULL) == COAP_INVALID_MID) + coap_log_warn("coap_handle_dgram: error sending response\n"); + } + coap_delete_pdu_lkd(pdu); +#if NO_SYS == 0 + sys_sem_signal(&coap_io_timeout_sem); +#endif /* NO_SYS == 0 */ + return; + } else { + goto error; + } } coap_lock_lock(session->context, return); coap_dispatch(session->context, session, pdu); @@ -220,6 +242,9 @@ error: if (session) coap_send_rst_lkd(session, pdu); coap_delete_pdu_lkd(pdu); +#if NO_SYS == 0 + sys_sem_signal(&coap_io_timeout_sem); +#endif /* NO_SYS == 0 */ return; } #endif /* ! COAP_CLIENT_SUPPORT */ @@ -289,12 +314,30 @@ coap_udp_recvs(void *arg, struct udp_pcb *upcb, struct pbuf *p, coap_session_new_dtls_session(session, now); pbuf_free(p); } else { + coap_opt_filter_t error_opts; + pdu = coap_pdu_from_pbuf(p); if (!pdu) goto error; - if (!coap_pdu_parse(ep->proto, p->payload, p->len, pdu)) { - goto error; + coap_option_filter_clear(&error_opts); + if (!coap_pdu_parse2(ep->proto, p->payload, p->len, pdu, &error_opts)) { + coap_handle_event_lkd(ep->context, COAP_EVENT_BAD_PACKET, session); + coap_log_warn("discard malformed PDU\n"); + if (error_opts.mask && COAP_PDU_IS_REQUEST(pdu)) { + coap_pdu_t *response = + coap_new_error_response(pdu, + COAP_RESPONSE_CODE(402), &error_opts); + if (!response) { + coap_log_warn("coap_handle_dgram: cannot create error response\n"); + } else { + if (coap_send_internal(session, response, NULL) == COAP_INVALID_MID) + coap_log_warn("coap_handle_dgram: error sending response\n"); + } + goto cleanup; + } else { + goto error; + } } coap_dispatch(ep->context, session, pdu); } @@ -317,9 +360,13 @@ error: */ if (session && pdu) coap_send_rst_lkd(session, pdu); +cleanup: coap_delete_pdu_lkd(pdu); coap_free_packet(packet); coap_lock_unlock(ep->context); +#if NO_SYS == 0 + sys_sem_signal(&coap_io_timeout_sem); +#endif /* NO_SYS == 0 */ return; } diff --git a/src/coap_net.c b/src/coap_net.c index bf1d2d0d..78466589 100644 --- a/src/coap_net.c +++ b/src/coap_net.c @@ -2499,9 +2499,22 @@ coap_read_session(coap_context_t *ctx, coap_session_t *session, coap_tick_t now) p += n; bytes_read -= n; if (n == len) { + coap_opt_filter_t error_opts; + + coap_option_filter_clear(&error_opts); if (coap_pdu_parse_header(session->partial_pdu, session->proto) - && coap_pdu_parse_opt(session->partial_pdu)) { + && coap_pdu_parse_opt(session->partial_pdu, &error_opts)) { coap_dispatch(ctx, session, session->partial_pdu); + } else if (error_opts.mask) { + coap_pdu_t *response = + coap_new_error_response(session->partial_pdu, + COAP_RESPONSE_CODE(402), &error_opts); + if (!response) { + coap_log_warn("coap_read_session: cannot create error response\n"); + } else { + if (coap_send_internal(session, response, NULL) == COAP_INVALID_MID) + coap_log_warn("coap_read_session: error sending response\n"); + } } coap_delete_pdu_lkd(session->partial_pdu); session->partial_pdu = NULL; @@ -2824,6 +2837,7 @@ coap_handle_dgram(coap_context_t *ctx, coap_session_t *session, uint8_t *msg, size_t msg_len) { coap_pdu_t *pdu = NULL; + coap_opt_filter_t error_opts; assert(COAP_PROTO_NOT_RELIABLE(session->proto)); if (msg_len < 4) { @@ -2844,10 +2858,25 @@ coap_handle_dgram(coap_context_t *ctx, coap_session_t *session, if (!pdu) goto error; - if (!coap_pdu_parse(session->proto, msg, msg_len, pdu)) { + coap_option_filter_clear(&error_opts); + if (!coap_pdu_parse2(session->proto, msg, msg_len, pdu, &error_opts)) { coap_handle_event_lkd(session->context, COAP_EVENT_BAD_PACKET, session); coap_log_warn("discard malformed PDU\n"); - goto error; + if (error_opts.mask && COAP_PDU_IS_REQUEST(pdu)) { + coap_pdu_t *response = + coap_new_error_response(pdu, + COAP_RESPONSE_CODE(402), &error_opts); + if (!response) { + coap_log_warn("coap_handle_dgram: cannot create error response\n"); + } else { + if (coap_send_internal(session, response, NULL) == COAP_INVALID_MID) + coap_log_warn("coap_handle_dgram: error sending response\n"); + } + coap_delete_pdu_lkd(pdu); + return -1; + } else { + goto error; + } } coap_dispatch(ctx, session, pdu); diff --git a/src/coap_pdu.c b/src/coap_pdu.c index 94a609b3..0f4d9eae 100644 --- a/src/coap_pdu.c +++ b/src/coap_pdu.c @@ -590,6 +590,7 @@ coap_option_check_repeatable(coap_option_num_t number) { case COAP_OPTION_URI_PATH: case COAP_OPTION_URI_QUERY: case COAP_OPTION_LOCATION_QUERY: + case COAP_OPTION_Q_BLOCK2: case COAP_OPTION_RTAG: break; /* Protest at the known non-repeatable options and ignore them */ @@ -602,6 +603,8 @@ coap_option_check_repeatable(coap_option_num_t number) { case COAP_OPTION_MAXAGE: case COAP_OPTION_HOP_LIMIT: case COAP_OPTION_ACCEPT: + case COAP_OPTION_Q_BLOCK1: + case COAP_OPTION_EDHOC: case COAP_OPTION_BLOCK2: case COAP_OPTION_BLOCK1: case COAP_OPTION_SIZE2: @@ -969,18 +972,16 @@ next_option_safe(coap_opt_t **optp, size_t *length, uint16_t *max_opt) { assert(length); optsize = coap_opt_parse(*optp, *length, &option); - if (optsize) { - assert(optsize <= *length); + assert(optsize <= *length); - /* signal an error if this option would exceed the - * allowed number space */ - if (*max_opt + option.delta > COAP_MAX_OPT) { - return 0; - } - *max_opt += option.delta; - *optp += optsize; - *length -= optsize; + /* signal an error if this option would exceed the + * allowed number space */ + if ((uint32_t)(*max_opt) + option.delta > COAP_MAX_OPT) { + return 0; } + *max_opt += option.delta; + *optp += optsize; + *length -= optsize; return optsize; } @@ -1252,10 +1253,18 @@ coap_pdu_parse_opt_base(coap_pdu_t *pdu, uint16_t len) { if (len > 2) res = 0; break; + case COAP_OPTION_Q_BLOCK1: + if (len > 3) + res = 0; + break; case COAP_OPTION_LOCATION_QUERY: if (len > 255) res = 0; break; + case COAP_OPTION_EDHOC: + if (len != 0) + res = 0; + break; case COAP_OPTION_BLOCK2: if (len > 3) res = 0; @@ -1268,6 +1277,10 @@ coap_pdu_parse_opt_base(coap_pdu_t *pdu, uint16_t len) { if (len > 4) res = 0; break; + case COAP_OPTION_Q_BLOCK2: + if (len > 3) + res = 0; + break; case COAP_OPTION_PROXY_URI: if (len < 1 || len > 1034) res = 0; @@ -1332,9 +1345,11 @@ write_char(char **obp, size_t *len, int c, int printable) { } int -coap_pdu_parse_opt(coap_pdu_t *pdu) { +coap_pdu_parse_opt(coap_pdu_t *pdu, coap_opt_filter_t *error_opts) { int good = 1; + coap_option_filter_clear(error_opts); + /* sanity checks */ if (pdu->code == 0) { if (pdu->used_size != 0 || pdu->e_token_length) { @@ -1369,6 +1384,7 @@ coap_pdu_parse_opt(coap_pdu_t *pdu) { coap_log_debug("coap_pdu_parse: %d.%02d: offset %u malformed option\n", pdu->code >> 5, pdu->code & 0x1F, (int)(opt_last - pdu->token - pdu->e_token_length)); + coap_option_filter_set(error_opts, pdu->max_opt); good = 0; break; } @@ -1379,6 +1395,7 @@ coap_pdu_parse_opt(coap_pdu_t *pdu) { pdu->code >> 5, pdu->code & 0x1F, (int)(opt_last - pdu->token - pdu->e_token_length), pdu->max_opt, len); + coap_option_filter_set(error_opts, pdu->max_opt); good = 0; } } @@ -1463,6 +1480,17 @@ coap_pdu_parse(coap_proto_t proto, const uint8_t *data, size_t length, coap_pdu_t *pdu) { + coap_opt_filter_t error_opts; + + return coap_pdu_parse2(proto, data, length, pdu, &error_opts); +} + +int +coap_pdu_parse2(coap_proto_t proto, + const uint8_t *data, + size_t length, + coap_pdu_t *pdu, + coap_opt_filter_t *error_opts) { size_t hdr_size; if (length == 0) @@ -1478,7 +1506,7 @@ coap_pdu_parse(coap_proto_t proto, memcpy(pdu->token - hdr_size, data, length); pdu->hdr_size = (uint8_t)hdr_size; pdu->used_size = length - hdr_size; - return coap_pdu_parse_header(pdu, proto) && coap_pdu_parse_opt(pdu); + return coap_pdu_parse_header(pdu, proto) && coap_pdu_parse_opt(pdu, error_opts); } size_t