blocks_delete_entry() moved used - i entries instead of used - i - 1
when deleting a single-block range that is not the last, reading one
coap_lg_range past the live set. Match the sibling branch below it.
oscore_decode_option_value() read opt_value[0] before the early return
for a zero-length (empty) option, so an empty OSCORE option dereferenced
a byte the option does not own. Move the read after the length check.
coap_oscore_decrypt_pdu() only checked encrypt_len == 0 before computing
encrypt_len - tag_len (size_t), so a protected payload shorter than the
tag underflowed to a huge value used for plain_pdu->used_size and the
AEAD output buffer size. This is reached before the payload is
authenticated.
Reject a payload that cannot hold the tag plus the inner Code byte up
front and handle it like any other decryption failure: a request is
answered with 4.00 "Decryption failed" and the replay state is rolled
back, so an unauthenticated short request cannot advance the replay
window. Also guard the subtraction in cose_encrypt0_decrypt().
Add a regression test for every payload length from one byte through
the tag length.
oscore_validate_sender_seq() moves the replay window on before the
message has been decrypted, and oscore_roll_back_seq() is meant to undo
that when decryption fails. It only restored last_seq and
sliding_window when the saved values were non zero, and never restored
initial_state. For a recipient context still in its initial state
(rfc8613_b_1_2 false), or one whose last_seq is 0, a single request
with a forged Partial IV and an invalid tag left last_seq at the
sender's chosen value, so the genuine peer's following requests were
rejected as outside the replay window.
The client did not roll back at all when a response failed decryption,
and the server called oscore_update_seq_num_cb() with the new window
before decrypting the request, so credential storage could be updated
with a forged sequence number even when the in memory window was
restored.
Record whether there is a window update to undo and restore
initial_state with it, roll back on response decryption failure as
well, and only call oscore_update_seq_num_cb() once the request has
been decrypted.
Add a unit test.
Handling a received PDU can disconnect the session, for example when
the response cannot be sent and coap_send_internal() calls
coap_session_disconnected_lkd(), which closes session->tls and frees
it. After coap_handle_dgram() returned, coap_dtls_receive() in the
wolfSSL backend still cleared the pdu pointer in the freed TLS data,
and the OpenSSL backend read it through the already freed BIO.
Clear the pdu pointer before passing the decrypted data to
coap_handle_dgram() and return its result directly, as the GnuTLS
backend already does, so the TLS data is never touched once the PDU
has been handed on. Do the same before the DTLS connected event and
l_establish() on handshake completion. Mbed TLS checked session->tls
for NULL instead; give it the same structure.
Add a unit test where the server's response write fails.
coap_notify_observers() walked the subscribers with LL_FOREACH_SAFE(),
which holds on to the next subscription while the current one is
notified. The request handler runs with the lock released, and a
notification that cannot be sent disconnects the session, which deletes
all of that session's subscriptions. When the next subscription was one
of those, the walk carried on with freed memory.
Mark the subscriptions to visit at the start of the walk and look up the
next marked one from the head of the list after each notification, so
nothing deleted during a notification is used afterwards.
Subscriptions added during the walk are not notified in that pass, as
before.
Add a unit test.
set_ciphersuites() builds the PSK, PKI and EC JPAKE cipher suite lists
in two passes, one counting and one filling, each with its own copy of
the selection rules, and then zero terminated the PSK and PKI lists
only. The EC JPAKE list is handed to mbedtls_ssl_conf_ciphersuites(),
which reads it up to the 0 entry, so an EC JPAKE client took whatever
was in the last slot as the next cipher suite and could read past the
end of the allocation when writing its ClientHello.
Decide which lists a cipher suite goes in with one function used by
both passes, and zero terminate every list, so the counts, contents and
terminators of the lists can no longer disagree.
Add a unit test.
pki_sni_callback() kept the PKI SNI cache as one array of entries that
hold the CA chain, certificate and key by value, and grew it with
realloc() for each new SNI. The certificate and key of an entry are
handed to the handshake with mbedtls_ssl_set_hs_ca_chain() and
mbedtls_ssl_set_hs_own_cert(), so when a second client asked for a new
SNI while another handshake was still running, that handshake was left
using freed memory.
Allocate each entry on its own and keep an array of pointers to them,
so an entry stays where it is for the life of the context. Also free
the partly set up credentials when setting up a new entry fails.
Add a unit test.
When a block-wise body is passed on as a single body, its length was taken
from total_len, which tracks the peer's Size2 / Size1 rather than the data
actually received.
Client Q-Block2: total_len was raised to a larger Size2 in a later block
before the Size2 mismatch check, so the check never fired for that case,
and nothing tied the last block to Size2. The body handed to the
application could then be longer than the data buffer behind it, or
include its uninitialised tail.
Server Block1 / Q-Block1: total_len kept a Size1 that was larger than
where the last block ended, so the body included the uninitialised tail
of the buffer allocated for the Size1 estimate.
Now the last block (M bit not set) defines the body length: for Q-Block2
and Q-Block1 it must end at Size2 / Size1 (RFC9177 requires these to be the
full size), and for Block1 / Block2 it replaces the Size2 / Size1 estimate
(RFC7959 4). A new total_len_set flag records that the last block has set
total_len, after which a block that ends past it, or another last block
that ends elsewhere, is rejected instead of changing the length.
Add unit tests to tests/test_block.c.
For a request set up with coap_add_data_large_request_app(), the data
is provided by the application's get_func and there is no data pointer.
Three places still took the body from that NULL pointer, so
coap_add_data() was passed a NULL pointer:
- check_freshness(), re-sending the next block with the Echo option
from a 4.01 response.
- coap_cancel_observe_lkd(), re-adding the observe request's body to
the cancel request.
- coap_add_data_large_internal(), when the application sets a Block1
option with a non-zero block number to send a single block.
Use get_func in all of these, as the other places that send a block of
an lg_xmit already do. The single block and the whole-body cases now
share add_get_func_data(), which also checks the allocation and no
longer leaks the buffer when get_func fails.
Add unit tests.
With track_client_session set, an incoming session gets its own proxy
entry for each upstream server it uses. coap_proxy_remove_association()
returned after the first entry tracking the session, so the proxy
requests of that session held in later entries were left in place when
the session was deleted, and their upstream sessions were not released.
A later upstream response was then forwarded to the freed session.
Carry on with the remaining entries instead of returning.
Add a unit test.
Reduce value of partial timeout if there are a lot of outstanding
large block receipts that are not active.
Add function coap_context_set_max_bodies_ram() to define an upper memory
limit usage across all active body receipts.
Examples are only built when the library mode they depend on is enabled,
otherwise their link step fails with undefined references. The tiny example
is gated on client mode only.
Signed-off-by: warter666 <271496918+warter666@users.noreply.github.com>
`coap_proxy_get_add_list_entry(incoming, ongoing, ...)` dereferences `ongoing->context` when `incoming == NULL`. Current in-tree callers always pass exactly one non-NULL session, but the function is a public-ish internal helper and a future caller passing both NULL would dereference NULL immediately. cppcheck flags exactly this (`nullPointer` on `ongoing`).
This adds an explicit early `return NULL` when both arguments are NULL - defensive only, no behavior change for existing callers.
Signed-off-by: warter666 <121139223+Warterkk@users.noreply.github.com>
Three format-string/argument type mismatches in `src/coap_debug.c`, all reported by `-Wall -Wextra` (fatal under -Werror):
1. `msg_code_string()`: `"%u.%02u"` receives `int` expressions (uint16_t promotes to int) - cast both to `unsigned`.
2. `print_content_format()`: `"%d"` receives `unsigned int format_type` - use `%u`.
3. block-size printing: `"%u"` receives the `int` expression `1 << (...)` - make the shift and cast unsigned.
No behavior change on any realistic input; only the int-vs-unsigned variance is fixed.
Signed-off-by: warter666 <121139223+Warterkk@users.noreply.github.com>
With `-DENABLE_CLIENT_MODE=OFF` (i.e. `COAP_CLIENT_SUPPORT=0`), `full_match()` in `src/coap_block.c` is defined but its only caller is compiled out, producing `-Wunused-function` (fatal in -Werror builds):
```
src/coap_block.c:479:1: error: unused function 'full_match' [-Werror=-Wunused-function]
```
Wrap the static-inline definition in the same `#if COAP_CLIENT_SUPPORT` guard as its caller.
Signed-off-by: warter666 <121139223+Warterkk@users.noreply.github.com>
In `coap_ws_rd_http_header_client()` the first header (the HTTP status line) is split by `coap_ws_split_rd_header()`, which returns NULL when the buffered line contains no space/tab separator. The second call site checks for NULL, but the first one does not:
```c
value = coap_ws_split_rd_header(session);
if (strcmp((char *)ws->http_hdr, "HTTP/1.1") != 0 ||
atoi(value) != 101) {
```
If the HTTP header buffer is exactly `"HTTP/1.1"` (no trailing space), `strcmp()` returns 0, the `||` short-circuit does not apply, and `atoi(NULL)` is called - a remote crash in the WS client handshake when a malicious or broken server sends a truncated status line.
Fix: return 0 when `value` is NULL, mirroring the second call site.
The library builds cleanly after the change (cmake -DENABLE_DTLS=OFF, gcc/clang, 0 warnings).
Signed-off-by: warter666 <121139223+Warterkk@users.noreply.github.com>
When a message spans multiple mbedTLS records, coap_tls_write() returns
only the final mbedtls_ssl_write() result despite sending the entire
message. Its caller treats the PDU as partially written and queues an
already transmitted suffix.
Update the successful return value to the accumulated byte count.
Preserve the existing nonblocking retry and fatal-error handling.
Validated with native mbedTLS 3.6.2 loopback transfers and a focused
ASan/UBSan harness covering record boundaries and WANT_READ/WANT_WRITE.
TCP BERT payloads can exceed the 1024-byte callback scratch buffer. Allocate a temporary buffer sized to the requested chunk, pass it to the large-data callback, and release it after the PDU has copied the data. Preserve the stack buffer for regular block transfers and report allocation failures explicitly.
Packets from the same peer can overwrite a session's local address with a
multicast destination. Save the address when an observation is created or
renewed and restore it before generating notifications, so deferred responses
snapshot the correct address instead of being dropped as invalid multicast.
Add regression coverage for CON and NON observations, including renewals and
the local address captured by a notification's async response.
Validation: 215 unit tests passed (1712 assertions).
A request handler that leaves the response code unset asks for no
response to be sent. That is honoured for a piggybacked ACK (the token
gets stripped, so an empty ACK goes out) and for a non-confirmable or
reliable response (dropped in no_response()), but not for a separate
response set up with coap_register_async().
There the response is turned into a CON, and a 0.00 CON was put on the
wire. An Empty message is a ping (RFC 7252 4.2), not a response, and the
PDU still carries the request's token, which RFC 7252 4.1 does not allow.
Being confirmable, it was then retransmitted up to MAX_RETRANSMIT times,
usually at a client port that had long gone away.
Drop it like the other two cases, delete it without logging it as a
dropped response since the application asked for this, and document in
coap_handler(3) that a separate response is not sent either.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Jon Shallow