From f073dcdb3deb2d98522721eeb545bd8a2ce9933a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6ren=20Tempel?= Date: Mon, 11 Jul 2022 22:55:33 +0200 Subject: [PATCH] gnrc_dhcpv6_client: Fix out-of-bounds access during option parsing The _parse_reply function iterates over the DHCPv6 message options twice but only performs sanity checks on the option length in the first iteration. As such, both loop iterations need to be identical. Unfortunately, there aren't without this commit as (1) they use different maximum length values and (2) the first iteration stops parsing as soon as it encounters a zero option while the second doesn't. As such, it is possible for out-of-bounds read to be performed by the second loop iteration. This commit fixes this. --- sys/net/application_layer/dhcpv6/client.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sys/net/application_layer/dhcpv6/client.c b/sys/net/application_layer/dhcpv6/client.c index 237a86a44a..219ca2cdef 100644 --- a/sys/net/application_layer/dhcpv6/client.c +++ b/sys/net/application_layer/dhcpv6/client.c @@ -988,6 +988,7 @@ static bool _parse_reply(uint8_t *rep, size_t len, uint8_t request_type) DEBUG("DHCPv6 client: packet too small or transaction ID wrong\n"); return false; } + len = orig_len - sizeof(dhcpv6_msg_t); for (dhcpv6_opt_t *opt = (dhcpv6_opt_t *)(&rep[sizeof(dhcpv6_msg_t)]); len > 0; len -= _opt_len(opt), opt = _opt_next(opt)) { if (len > orig_len) { @@ -1079,6 +1080,10 @@ static bool _parse_reply(uint8_t *rep, size_t len, uint8_t request_type) default: break; } + /* 0 option is used as an end marker, len can include bogus bytes */ + if (!byteorder_ntohs(opt->type)) { + break; + } } return true; }