Summary
I spotted the lack of a size check that may lead to a buffer overflow in RIOT source code at:
https://github.com/RIOT-OS/RIOT/blob/master/sys/net/application_layer/cord/lc/cord_lc.c#L218
Details
The _on_rd_init() function does not implement a size check before copying data to the _result_buf static buffer. If an attacker can craft a long enough payload, they could cause a buffer overflow.
Please refer to the marked line below:
static void _on_rd_init(const gcoap_request_memo_t *memo, coap_pkt_t *pdu,
const sock_udp_ep_t *remote)
{
(void)remote;
thread_flags_t flag = FLAG_NORSC;
if (memo->state == GCOAP_MEMO_RESP) {
unsigned ct = coap_get_content_type(pdu);
if (ct != COAP_FORMAT_LINK) {
DEBUG("cord_lc: error payload not in link format: %u\n", ct);
goto end;
}
if (pdu->payload_len == 0) {
DEBUG("cord_lc: error empty payload\n");
goto end;
}
memcpy(_result_buf, pdu->payload, pdu->payload_len); // VULN: lack of size check and potential buffer overflow
_result_buf_len = pdu->payload_len;
_result_buf[_result_buf_len] = '\0';
flag = FLAG_SUCCESS;
} else if (memo->state == GCOAP_MEMO_TIMEOUT) {
flag = FLAG_TIMEOUT;
}
end:
if (flag != FLAG_SUCCESS) {
_result_buf = NULL;
_result_buf_len = 0;
}
thread_flags_set(_waiter, flag);
}Note that the _on_lookup() function in the same file does implement such an explicit size check:
static void _on_lookup(const gcoap_request_memo_t *memo, coap_pkt_t *pdu,
const sock_udp_ep_t *remote)
{
(void)remote;
thread_flags_t flag = FLAG_ERR;
if (memo->state == GCOAP_MEMO_RESP) {
unsigned ct = coap_get_content_type(pdu);
if (ct != COAP_FORMAT_LINK) {
DEBUG("cord_lc: unsupported content format: %u\n", ct);
thread_flags_set(_waiter, flag);
}
if (pdu->payload_len == 0) {
flag = FLAG_NORSC;
thread_flags_set(_waiter, flag);
}
if (pdu->payload_len >= _result_buf_len) { // CHECK
flag = FLAG_OVERFLOW;
thread_flags_set(_waiter, flag);
}
memcpy(_result_buf, pdu->payload, pdu->payload_len);
memset(_result_buf + pdu->payload_len, 0,
_result_buf_len - pdu->payload_len);
_result_buf_len = pdu->payload_len;
flag = FLAG_SUCCESS;
} else if (memo->state == GCOAP_MEMO_TIMEOUT) {
flag = FLAG_TIMEOUT;
}
thread_flags_set(_waiter, flag);
}Impact
If the unchecked input above is attacker-controlled and crosses a security boundary, the impact of the buffer overflow vulnerability could range from denial of service to arbitrary code execution.
0xdea Reporter
Summary
I spotted the lack of a size check that may lead to a buffer overflow in RIOT source code at:
https://github.com/RIOT-OS/RIOT/blob/master/sys/net/application_layer/cord/lc/cord_lc.c#L218
Details
The
_on_rd_init()function does not implement a size check before copying data to the_result_bufstatic buffer. If an attacker can craft a long enough payload, they could cause a buffer overflow.Please refer to the marked line below:
Note that the
_on_lookup()function in the same file does implement such an explicit size check:Impact
If the unchecked input above is attacker-controlled and crosses a security boundary, the impact of the buffer overflow vulnerability could range from denial of service to arbitrary code execution.