dhcp6_parse_options() takes the SERVERID and CLIENTID option lengths
from the received ADVERTISE/REPLY and uses them unchecked:

- the SERVERID length is stored as server_uid_size and later copied into
  the fixed net_tx_packet buffer in dhcp6_send_request_packet() with no
  capacity check, so a large SERVERID overruns net_tx_packet (an
  out-of-bounds write);
- the CLIENTID length is passed straight to memcmp() against the fixed
  sm_params.duid buffer, so a length larger than it reads past the end
  (an out-of-bounds read).

Both are reachable by any on-link attacker that answers a DHCPv6 SOLICIT
during netboot; neither length is bounded against anything but the
received packet size.

Bound the SERVERID length to the RFC 8415 DUID maximum at parse time,
which also bounds the malloc() and the copy into the REQUEST for every
caller, and only compare a CLIENTID that is exactly the size of the
client DUID.

Fixes: a0245818f7f8 ("net: dhcp6: Add DHCPv6 (DHCP for IPv6)")
Signed-off-by: Shahriyar Jalayeri <[email protected]>
---
 net/dhcpv6.c | 12 ++++++++----
 net/dhcpv6.h |  3 +++
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/net/dhcpv6.c b/net/dhcpv6.c
index 640f089a2e1..a18e358137c 100644
--- a/net/dhcpv6.c
+++ b/net/dhcpv6.c
@@ -322,15 +322,19 @@ static void dhcp6_parse_options(uchar *rx_pkt, unsigned 
int len)
 
                switch (ntohs(option_hdr->option_id)) {
                case DHCP6_OPTION_CLIENTID:
-                       if (memcmp(option_ptr, sm_params.duid, option_len)
-                           != 0) {
-                               debug("CLIENT ID DOESN'T MATCH\n");
-                       } else {
+                       if (option_len == sizeof(sm_params.duid) &&
+                           !memcmp(option_ptr, sm_params.duid, option_len)) {
                                debug("CLIENT ID FOUND and MATCHES\n");
                                sm_params.rx_status.client_id_match = true;
+                       } else {
+                               debug("CLIENT ID DOESN'T MATCH\n");
                        }
                        break;
                case DHCP6_OPTION_SERVERID:
+                       if (option_len > DHCP6_DUID_MAX_LEN) {
+                               debug("SERVER ID too long\n");
+                               break;
+                       }
                        sm_params.rx_status.server_id_found = true;
                        sm_params.rx_status.server_uid_ptr = (uchar 
*)option_hdr;
                        sm_params.rx_status.server_uid_size = option_len +
diff --git a/net/dhcpv6.h b/net/dhcpv6.h
index d41a3c30615..b0207a9f2c9 100644
--- a/net/dhcpv6.h
+++ b/net/dhcpv6.h
@@ -37,6 +37,9 @@
 #define DUID_LL_SIZE           (sizeof(struct dhcp6_option_duid_ll) + ETH_ALEN)
 #define DUID_MAX_SIZE          DUID_LL_SIZE /* only supports DUID-LL currently 
*/
 
+/* RFC 8415 sec 11.1: a DUID is a 2-octet type plus at most 128 octets */
+#define DHCP6_DUID_MAX_LEN     130
+
 /* vendor-class-data to send in vendor clas option */
 #define DHCP6_VCI_STRING       "U-Boot"
 

-- 
2.43.0

Reply via email to