From 498e531e2f3a6dfa46b93a38fae5f9df169bac9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6ren=20Tempel?= Date: Thu, 22 Sep 2022 20:08:00 +0200 Subject: [PATCH] dhcpv6: don't treat zero option as an end-of-payload marker As far as I can tell, no DHCPv6 RFC specifies this option. The handling for the zero option was added in #17736 by @benpicco to fix issues encountered while trying to retrieve a DHCHPv6 lease. However, I strongly suspect that the zero option was encountered in this case due to an out-of-bounds read performed in RIOT's DHCPv6 client implementation (i.e. the option parsing loop read beyond the packet bounds). This issue was fixed in #18307 and I strongly suspect that it should also fix the issue @benpicco originally encountered in #17736. As such, I propose that we remove the if statement which treats the zero option as an end-of-payload marker. Fixes #18309 --- sys/net/application_layer/dhcpv6/client.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/sys/net/application_layer/dhcpv6/client.c b/sys/net/application_layer/dhcpv6/client.c index 219ca2cdef..0136cb7142 100644 --- a/sys/net/application_layer/dhcpv6/client.c +++ b/sys/net/application_layer/dhcpv6/client.c @@ -1017,10 +1017,6 @@ 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; - } } if ((cid == NULL) || (sid == NULL)) { DEBUG("DHCPv6 client: ADVERTISE does not contain either server ID " @@ -1080,10 +1076,6 @@ 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; }