Merge pull request #21087 from maribu/backport/2024.10/sys/net/nanoco…

…ap/buffer-overflow-separate-response

sys/net/nanocoap: fix buffer overflow in separate response handling [backport 2024.10]

examples/nanocoap_server/coap_handler.c

@@ -209,9 +209,13 @@ static ssize_t _separate_handler(coap_pkt_t *pkt, uint8_t *buf, size_t len, coap
         return coap_build_reply(pkt, COAP_CODE_SERVICE_UNAVAILABLE, buf, len, 0);
     }
 
-    puts("_separate_handler(): send ACK, schedule response");
+    if (nanocoap_server_prepare_separate(&_separate_ctx, pkt, context)) {
+        puts("_separate_handler(): failed to prepare context for separate response");
+        /* send a reset message, as we don't support large tokens here */
+        return coap_build_reply(pkt, 0, buf, len, 0);
+    }
 
-    nanocoap_server_prepare_separate(&_separate_ctx, pkt, context);
+    puts("_separate_handler(): send ACK, schedule response");
 
     event_timeout_ztimer_init(&event_timeout, ZTIMER_MSEC, EVENT_PRIO_MEDIUM,
                               &event_timed.super);

sys/include/net/nanocoap_sock.h

@@ -246,9 +246,15 @@ typedef struct {
  * @param[out]  ctx     Context information for separate response
  * @param[in]   pkt     CoAP packet to which the response will be generated
  * @param[in]   req     Context of the CoAP request
+ *
+ * @retval  0           Success
+ * @retval  -EOVERFLOW  Storing context would have overflown buffers in @p ctx
+ *                      (e.g. RFC 8974 (module `nanocoap_token_ext`) is in
+ *                      use and token too long)
+ * @retval  <0          Other error
  */
-void nanocoap_server_prepare_separate(nanocoap_server_response_ctx_t *ctx,
-                                      coap_pkt_t *pkt, const coap_request_ctx_t *req);
+int nanocoap_server_prepare_separate(nanocoap_server_response_ctx_t *ctx,
+                                     coap_pkt_t *pkt, const coap_request_ctx_t *req);
 
 /**
  * @brief   Send a separate response to a CoAP request

sys/net/application_layer/nanocoap/sock.c

@@ -1052,11 +1052,22 @@ void auto_init_nanocoap_server(void)
     nanocoap_server_start(&local);
 }
 
-void nanocoap_server_prepare_separate(nanocoap_server_response_ctx_t *ctx,
-                                    coap_pkt_t *pkt, const coap_request_ctx_t *req)
+int nanocoap_server_prepare_separate(nanocoap_server_response_ctx_t *ctx,
+                                     coap_pkt_t *pkt, const coap_request_ctx_t *req)
 {
-    ctx->tkl = coap_get_token_len(pkt);
-    memcpy(ctx->token, coap_get_token(pkt), ctx->tkl);
+    size_t tkl = coap_get_token_len(pkt);
+    if (tkl > sizeof(ctx->token)) {
+        DEBUG_PUTS("nanocoap: token too long for separate response ctx");
+        /* Legacy code may not check the return value. To still have somewhat
+         * sane behavior, we ask for no response for any response class.
+         * Getting no reply is certainly not ideal, but better than one without
+         * a matching token. */
+        memset(ctx, 0, sizeof(*ctx));
+        ctx->no_response = 0xff;
+        return -EOVERFLOW;
+    }
+    ctx->tkl = tkl;
+    memcpy(ctx->token, coap_get_token(pkt), tkl);
     memcpy(&ctx->remote, req->remote, sizeof(ctx->remote));
 #ifdef MODULE_SOCK_AUX_LOCAL
     assert(req->local);
@@ -1065,6 +1076,8 @@ void nanocoap_server_prepare_separate(nanocoap_server_response_ctx_t *ctx,
     uint32_t no_response = 0;
     coap_opt_get_uint(pkt, COAP_OPT_NO_RESPONSE, &no_response);
     ctx->no_response = no_response;
+
+    return 0;
 }
 
 int nanocoap_server_send_separate(const nanocoap_server_response_ctx_t *ctx,

tests/net/nanocoap_cli/Makefile

@@ -15,6 +15,7 @@ USEMODULE += gnrc_ipv6_default
 
 USEMODULE += nanocoap_sock
 USEMODULE += nanocoap_resources
+USEMODULE += fmt # scn_buf_hex() for shell cmd client_token
 
 # boards where basic nanocoap functionality fits, but no VFS
 LOW_MEMORY_BOARDS := \

tests/net/nanocoap_cli/nanocli_client.c

@@ -23,6 +23,7 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include "fmt.h"
 #include "net/coap.h"
 #include "net/gnrc/netif.h"
 #include "net/ipv6.h"