RIOT-OS/RIOT
RIOT-OS/RIOT
1
0
Fork 0
mirror of https://github.com/RIOT-OS/RIOT.git synced 2024-12-29 04:50:03 +01:00
Code Releases Activity

sys/net/nanocoap: fix UB when building hdr

Browse Source 
Some calls to `coap_build_hdr()` were done with the target buffer for
the header and the source buffer for the token overlapping:
They reuse the buffer that held the request to assemble the response in.
We cannot use `memcpy()` in this case to copy the token into the target
buffer, as source and destination would (fully) overlap.

This commit makes reusing the request buffer for the response a special
case: `memcpy()` is only used to copy the token if source and
destination address of the token differ.

An alternative fix would have been to use `memmove()` unconditionally.
But `memmove()` does not make any assumption about the layout of target
and source buffer, while we know that the token either will already be
at the right position (when reusing the request buffer for the response)
or be in a non-overlapping buffer (when generating a fresh token). This
approach is more efficient than `memmove()`.
This commit is contained in:
Marian Buschsieweke 2024-10-17 09:37:01 +02:00
parent 3706589959
commit 835571c0a7
No known key found for this signature in database
GPG Key ID: 758BD52517F79C41
2 changed files with 15 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
sys/include/net/nanocoap.h
View File

@ -1946,6 +1946,11 @@ ssize_t coap_block2_build_reply(coap_pkt_t *pkt, unsigned code,
* @param[in] code CoAP code (e.g., COAP_CODE_204, ...)
* @param[in] id CoAP request id
*
* @pre @p token is either not overlapping with the memory buffer
* @p hdr points to, or is already at the right offset (e.g.
* when building the response inside the buffer the contained
* the request).
*
* @returns length of resulting header
*/
ssize_t coap_build_hdr(coap_hdr_t *hdr, unsigned type, const void *token,

12
sys/net/application_layer/nanocoap/nanocoap.c
View File

@ -724,8 +724,16 @@ ssize_t coap_build_hdr(coap_hdr_t *hdr, unsigned type, const void *token,
memcpy(hdr + 1, &tkl_ext, tkl_ext_len);
}
if (token_len) {
memcpy(coap_hdr_data_ptr(hdr), token, token_len);
/* Some users build a response packet in the same buffer that contained
* the request. In this case, the argument token already points inside
* the target, or more specifically, it is already at the correct place.
* Having `src` and `dest` in `memcpy(dest, src, len)` overlap is
* undefined behavior, so have to treat this explicitly. We could use
* memmove(), but we know that either `src` and `dest` do not overlap
* at all, or fully. So we can be a bit more efficient here. */
void *token_dest = coap_hdr_data_ptr(hdr);
if (token_dest != token) {
memcpy(token_dest, token, token_len);
}
return sizeof(coap_hdr_t) + token_len + tkl_ext_len;