Summary
I spotted a typo in 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/gcoap/dns.c#L319-L325
I also spotted another potential buffer overflow in RIOT source code at:
https://github.com/RIOT-OS/RIOT/blob/master/sys/net/application_layer/gcoap/forward_proxy.c#L352
Details
The size check in the gcoap_dns_server_proxy_get() function contains a small typo that may lead to a buffer overflow in the subsequent strcpy(). In detail, the length of the _uri string is checked instead of the length of the _proxy string.
Please refer to the marked lines below:
ssize_t gcoap_dns_server_proxy_get(char *proxy, size_t proxy_len)
{
ssize_t res = 0;
mutex_lock(&_client_mutex);
if (_dns_server_uri_isset()) {
res = strlen(_uri); // VULN: typo, should be strlen(_proxy)
if (((size_t)res + 1) > proxy_len) {
/* account for trailing \0 */
res = -ENOBUFS;
}
else {
strcpy(proxy, _proxy); // VULN: potential buffer overflow
}
}
mutex_unlock(&_client_mutex);
return res;
}The _gcoap_forward_proxy_copy_options() function does not implement an explicit size check before copying data to the cep->req_etag buffer that is COAP_ETAG_LENGTH_MAX bytes long. If an attacker can craft input so that optlen becomes larger than COAP_ETAG_LENGTH_MAX, they can cause a buffer overflow.
Please refer to the marked line below:
static int _gcoap_forward_proxy_copy_options(coap_pkt_t *pkt,
coap_pkt_t *client_pkt,
client_ep_t *cep,
uri_parser_result_t *urip)
{
/* copy all options from client_pkt to pkt */
coap_optpos_t opt = {0, 0};
uint8_t *value;
bool uri_path_added = false;
bool etag_added = false;
for (int i = 0; i < client_pkt->options_len; i++) {
ssize_t optlen = coap_opt_get_next(client_pkt, &opt, &value, !i);
/* wrt to ETag option slack: we always have at least the Proxy-URI option in the client_pkt,
* so we should hit at least once (and it's opt_num is also >= COAP_OPT_ETAG) */
if (optlen >= 0) {
if (IS_USED(MODULE_NANOCOAP_CACHE) && !etag_added && (opt.opt_num >= COAP_OPT_ETAG)) {
static const uint8_t tmp[COAP_ETAG_LENGTH_MAX] = { 0 };
/* add slack to maybe add an ETag on stale cache hit later, as is done in
* gcoap_req_send() (which we circumvented in _gcoap_forward_proxy_via_coap()) */
if (coap_opt_add_opaque(pkt, COAP_OPT_ETAG, tmp, sizeof(tmp))) {
etag_added = true;
}
}
if (IS_USED(MODULE_NANOCOAP_CACHE) && opt.opt_num == COAP_OPT_ETAG) {
if (_cep_get_req_etag_len(cep) == 0) {
/* TODO: what to do on multiple ETags? */
_cep_set_req_etag_len(cep, (uint8_t)optlen);
#if IS_USED(MODULE_NANOCOAP_CACHE)
/* req_tag in cep is pre-processor guarded so we need to as well */
memcpy(cep->req_etag, value, optlen); // VULN: potential buffer overflow if optlen can become larger than COAP_ETAG_LENGTH_MAX
#endif
...Impact
If the input above is attacker-controlled and crosses a security boundary, the impact of the buffer overflow vulnerabilities could range from denial of service to arbitrary code execution.
0xdea Reporter
Summary
I spotted a typo in 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/gcoap/dns.c#L319-L325
I also spotted another potential buffer overflow in RIOT source code at:
https://github.com/RIOT-OS/RIOT/blob/master/sys/net/application_layer/gcoap/forward_proxy.c#L352
Details
The size check in the
gcoap_dns_server_proxy_get()function contains a small typo that may lead to a buffer overflow in the subsequentstrcpy(). In detail, the length of the_uristring is checked instead of the length of the_proxystring.Please refer to the marked lines below:
The
_gcoap_forward_proxy_copy_options()function does not implement an explicit size check before copying data to thecep->req_etagbuffer that isCOAP_ETAG_LENGTH_MAXbytes long. If an attacker can craft input so thatoptlenbecomes larger thanCOAP_ETAG_LENGTH_MAX, they can cause a buffer overflow.Please refer to the marked line below:
Impact
If the input above is attacker-controlled and crosses a security boundary, the impact of the buffer overflow vulnerabilities could range from denial of service to arbitrary code execution.