From ffbc5f7de4f68acb5a472848e6fab80f41c4f1a0 Mon Sep 17 00:00:00 2001 From: Franc Urbanc Date: Thu, 1 Oct 2026 09:20:19 +0200 Subject: [PATCH 1/5] Network: fix parsing of malformed DHCP options --- Components/Network/Source/net_dhcp_client.c | 235 ++++++++++++++---- Components/Network/Source/net_dhcp_client.h | 8 +- Components/Network/Source/net_evr.h | 2 +- Documentation/Doxygen/Network/src/net_evr.txt | 2 +- 4 files changed, 192 insertions(+), 55 deletions(-) diff --git a/Components/Network/Source/net_dhcp_client.c b/Components/Network/Source/net_dhcp_client.c index dc82d3b1..06ae05b3 100644 --- a/Components/Network/Source/net_dhcp_client.c +++ b/Components/Network/Source/net_dhcp_client.c @@ -1,6 +1,6 @@ /*------------------------------------------------------------------------------ * MDK Middleware - Component ::Network - * Copyright (c) 2004-2025 Arm Limited (or its affiliates). All rights reserved. + * Copyright (c) 2004-2026 Arm Limited (or its affiliates). All rights reserved. *------------------------------------------------------------------------------ * Name: net_dhcp_client.c * Purpose: Dynamic Host Configuration Client @@ -43,6 +43,13 @@ static bool is_selected (NET_DHCP_CFG *h, const uint8_t *ip_addr); static uint32_t proc_opt_sel (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t index); static uint32_t proc_opt_req (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t index); static uint32_t proc_opt_renew (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t index); +static void parse_sel_opts (NET_DHCP_CFG *h, + const uint8_t *buf, uint32_t start, uint32_t end); +static void parse_req_opts (NET_DHCP_CFG *h, + const uint8_t *buf, uint32_t start, uint32_t end); +static void parse_renew_opts (NET_DHCP_CFG *h, + const uint8_t *buf, uint32_t start, uint32_t end); +static uint32_t validate_index (uint32_t index, const uint8_t *buf, uint32_t len); static void assign_auto_ip (NET_DHCP_CFG *h); #ifdef Network_Debug_STDIO static void debug_info (const uint8_t *opt); @@ -694,7 +701,7 @@ static void dhcp_send_message (NET_DHCP_CFG *h, uint8_t msg_type) { sz = index + 1; if (sz < 300) sz = 300; - + /* Send unicast message when server's IP is available */ addr.addr_type = NET_ADDR_IP4; addr.port = DHCP_SERVER_PORT; @@ -754,7 +761,7 @@ static uint32_t dhcp_listener (int32_t socket, const NET_ADDR *addr, return (false); } /* Initial correctness checking */ - if (len < 236) { + if (len < 240) { ERRORF (DHCP,"Process %s, Frame too short\n",h->If->Name); EvrNetDHCP_FrameTooShort (h->If->Id, len, 236); return (false); @@ -800,10 +807,14 @@ static uint32_t dhcp_listener (int32_t socket, const NET_ADDR *addr, EvrNetDHCP_ClientState (h->If->Id, DHCP_STATE_SELECTING); /* Go through all parameters and try to find if this is DHCP_OFFER */ /* Search for the SERVER_IDENTIFIER option (a server's IP address) */ - for (index = 240; index < len; ) { + index = validate_index (240, buf, len); + while (index < len) { switch (buf[index]) { case DHCP_OPT_MSG_TYPE: /* Check message type */ + if (buf[index+1] != 1U) { + break; + } if (buf[index+2] != DHCP_OFFER) { DEBUGF (DHCP," Discarded, Not DHCP_OFFER message\n"); EvrNetDHCP_InvalidMessageType (h->If->Id, buf[index+2], DHCP_OFFER); @@ -815,27 +826,26 @@ static uint32_t dhcp_listener (int32_t socket, const NET_ADDR *addr, case DHCP_OPT_OVERLOAD: /* Overloaded message, read overload value */ - overload = buf[index+2]; + if (buf[index+1] == 1U) { + overload = buf[index+2]; + } break; } index = proc_opt_sel (h, buf, index); + index = validate_index (index, buf, len); } /* For overload message, 'sname' and 'file' fields hold options */ if (overload & 0x01) { /* Process options overloaded to 'file' field */ DEBUGF (DHCP," Processing Overload 'file' options\n"); EvrNetDHCP_FileOverloadOptions (h->If->Id, DHCP_OFFER); - for (index = 108; index < (108+128); ) { - index = proc_opt_sel (h, buf, index); - } + parse_sel_opts (h, buf, BOOTP_FILE_OFFS, BOOTP_FILE_END); } if (overload & 0x02) { /* Process options overloaded to 'sname' field */ DEBUGF (DHCP," Processing Overload 'sname' options\n"); EvrNetDHCP_SnameOverloadOptions (h->If->Id, DHCP_OFFER); - for (index = 44; index < (44+64); ) { - index = proc_opt_sel (h, buf, index); - } + parse_sel_opts (h, buf, BOOTP_SNAME_OFFS, BOOTP_SNAME_END); } /* Processed all, got what we were looking for ? */ if (__ALIGNED_UINT32(ctrl->SrvAddr) == 0) { @@ -906,11 +916,15 @@ static uint32_t dhcp_listener (int32_t socket, const NET_ADDR *addr, } /* Go through param list and process them. We expect to get */ /* all the params here and that this is DHCP_ACK message. */ - for (index = 240; index < len; ) { + index = validate_index (240, buf, len); + while (index < len) { switch (buf[index]) { case DHCP_OPT_MSG_TYPE: /* If this is DHCP_ACK we can switch state and assume */ /* that the parameters are OK. */ + if (buf[index+1] != 1U) { + break; + } if (buf[index+2] != DHCP_ACK) { DEBUGF (DHCP," Discarded, Not DHCP_ACK message\n"); EvrNetDHCP_InvalidMessageType (h->If->Id, buf[index+2], DHCP_ACK); @@ -929,27 +943,26 @@ static uint32_t dhcp_listener (int32_t socket, const NET_ADDR *addr, case DHCP_OPT_OVERLOAD: /* Overloaded message, read overload value */ - overload = buf[index+2]; + if (buf[index+1] == 1U) { + overload = buf[index+2]; + } break; } index = proc_opt_req (h, buf, index); + index = validate_index (index, buf, len); } /* For overload message, 'sname' and 'file' fields hold options */ if (overload & 0x01) { /* Process options overloaded to 'file' field. */ DEBUGF (DHCP," Processing Overload 'file' options\n"); EvrNetDHCP_FileOverloadOptions (h->If->Id, DHCP_ACK); - for (index = 108; index < (108+128); ) { - index = proc_opt_req (h, buf, index); - } + parse_req_opts (h, buf, BOOTP_FILE_OFFS, BOOTP_FILE_END); } if (overload & 0x02) { /* Process options overloaded to 'sname' field */ DEBUGF (DHCP," Processing Overload 'sname' options\n"); EvrNetDHCP_SnameOverloadOptions (h->If->Id, DHCP_ACK); - for (index = 44; index < (44+64); ) { - index = proc_opt_req (h, buf, index); - } + parse_req_opts (h, buf, BOOTP_SNAME_OFFS, BOOTP_SNAME_END); } DEBUGF (DHCP," Next State-BOUND\n"); EvrNetDHCP_NextState (h->If->Id, DHCP_STATE_BOUND); @@ -977,9 +990,13 @@ ack: if (!is_selected (h, addr->addr)) { EvrNetDHCP_ServerAddressNotSelected (h->If->Id, addr->addr); return (false); } - for (index = 240; index < len; ) { + index = validate_index (240, buf, len); + while (index < len) { switch (buf[index]) { case DHCP_OPT_MSG_TYPE: + if (buf[index+1] != 1U) { + break; + } if (buf[index+2] == DHCP_NAK) { DEBUGF (DHCP," Received DHCP_NAK message\n"); DEBUGF (DHCP," Next State-INIT\n"); @@ -1001,27 +1018,26 @@ ack: if (!is_selected (h, addr->addr)) { case DHCP_OPT_OVERLOAD: /* Overloaded message, read overload value */ - overload = buf[index+2]; + if (buf[index+1] == 1U) { + overload = buf[index+2]; + } break; } index = proc_opt_renew (h, buf, index); + index = validate_index (index, buf, len); } /* For overload message, 'sname' and 'file' fields hold options */ if (overload & 0x01) { /* Process options overloaded to 'file' field */ DEBUGF (DHCP," Processing Overload 'file' options\n"); EvrNetDHCP_FileOverloadOptions (h->If->Id, DHCP_ACK); - for (index = 108; index < (108+128); ) { - index = proc_opt_renew (h, buf, index); - } + parse_renew_opts (h, buf, BOOTP_FILE_OFFS, BOOTP_FILE_END); } if (overload & 0x02) { /* Process options overloaded to 'sname' field */ DEBUGF (DHCP," Processing Overload 'sname' options\n"); EvrNetDHCP_SnameOverloadOptions (h->If->Id, DHCP_ACK); - for (index = 44; index < (44+64); ) { - index = proc_opt_renew (h, buf, index); - } + parse_renew_opts (h, buf, BOOTP_SNAME_OFFS, BOOTP_SNAME_END); } DEBUGF (DHCP," Next State-BOUND\n"); EvrNetDHCP_NextState (h->If->Id, DHCP_STATE_BOUND); @@ -1057,7 +1073,23 @@ static bool is_selected (NET_DHCP_CFG *h, const uint8_t *ip_addr) { } /** - \brief Process DHCP option in state SELECTING. + \brief Parse overloaded options in SELECTING state. + \param[in] h DHCP instance handle. + \param[in] buf buffer containing DHCP options. + \param[in] start index of the first option to parse. + \param[in] end end position of the options. +*/ +static void parse_sel_opts (NET_DHCP_CFG *h, + const uint8_t *buf, uint32_t start, uint32_t end) { + uint32_t index = validate_index (start, buf, end); + while (index < end) { + index = proc_opt_sel (h, buf, index); + index = validate_index (index, buf, end); + } +} + +/** + \brief Process DHCP option in SELECTING state. \param[in] h DHCP instance handle. \param[in] buf buffer containing DHCP options. \param[in] index position of an option to process. @@ -1067,6 +1099,10 @@ static uint32_t proc_opt_sel (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t inde switch (buf[index]) { case DHCP_OPT_SERVER_ID: /* Read Server Identifier */ + if (buf[index+1] != 4U) { + /* Invalid option length, ignore */ + break; + } net_addr4_copy (ctrl->SrvAddr, &buf[index+2]); DEBUG_INF2 ("Server ID",ctrl->SrvAddr); EvrNetDHCP_ViewServerId (h->If->Id, ctrl->SrvAddr); @@ -1074,10 +1110,6 @@ static uint32_t proc_opt_sel (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t inde case DHCP_OPT_PAD: return (index + 1); - - case DHCP_OPT_END: - /* End of options */ - return (0xFFFFFFFF); } /* Calculate index of next option */ index += buf[index+1] + 2; @@ -1085,7 +1117,23 @@ static uint32_t proc_opt_sel (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t inde } /** - \brief Process DHCP option in state REQUESTING. + \brief Parse overloaded options in REQUESTING state. + \param[in] h DHCP instance handle. + \param[in] buf buffer containing DHCP options. + \param[in] start index of the first option to parse. + \param[in] end end position of the options. +*/ +static void parse_req_opts (NET_DHCP_CFG *h, + const uint8_t *buf, uint32_t start, uint32_t end) { + uint32_t index = validate_index (start, buf, end); + while (index < end) { + index = proc_opt_req (h, buf, index); + index = validate_index (index, buf, end); + } +} + +/** + \brief Process DHCP option in REQUESTING state. \param[in] h DHCP instance handle. \param[in] buf buffer containing DHCP options. \param[in] index position of an option to process. @@ -1093,9 +1141,14 @@ static uint32_t proc_opt_sel (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t inde */ static uint32_t proc_opt_req (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t index) { uint32_t tmp; + char *dbuf; switch (buf[index]) { case DHCP_OPT_SUBNET_MASK: + if (buf[index+1] != 4U) { + /* Invalid option length, ignore */ + break; + } net_addr4_copy (LocM.NetMask, &buf[index+2]); DEBUG_INF2 ("Net mask",LocM.NetMask); EvrNetDHCP_ViewNetMask (h->If->Id, LocM.NetMask); @@ -1103,6 +1156,10 @@ static uint32_t proc_opt_req (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t inde case DHCP_OPT_ROUTER: /* Read Default Gateway IP address */ + if (buf[index+1] < 4U || (buf[index+1] & 3U) != 0U) { + /* Option length must be a non-zero multiple of 4 */ + break; + } net_addr4_copy (LocM.DefGW, &buf[index+2]); DEBUG_INF2 ("Gateway",LocM.DefGW); EvrNetDHCP_ViewGatewayAddress (h->If->Id, LocM.DefGW); @@ -1110,6 +1167,10 @@ static uint32_t proc_opt_req (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t inde case DHCP_OPT_DNS_SERVER: /* Read primary and secondary DNS Server */ + if (buf[index+1] < 4U || (buf[index+1] & 3U) != 0U) { + /* Option length must be a non-zero multiple of 4 */ + break; + } net_addr4_copy (LocM.PriDNS, &buf[index+2]); net_addr4_copy (LocM.SecDNS, (buf[index+1] < 8) ? net_addr_unspec : &buf[index+6]); DEBUG_INF2 ("Prim. DNS",LocM.PriDNS); @@ -1122,17 +1183,23 @@ static uint32_t proc_opt_req (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t inde break; case DHCP_OPT_LEASE_TIME: + if (buf[index+1] != 4U) { + break; + } tmp = get_u32 (&buf[index+2]); /* Time calculation from RFC! These values can */ /* also be received directly in options. */ - ctrl->t1 = 5 * tmp / 10; /* 0.5*tmp */ - ctrl->t2 = 7 * tmp / 8; /* 0.875*tmp */ + ctrl->t1 = tmp / 2U; /* T1 = 0.5*tmp */ + ctrl->t2 = tmp - tmp / 8U; /* T2 = 0.875*tmp */ DEBUGF (DHCP," Lease time: %d sec\n",tmp); EvrNetDHCP_ViewLeaseTime (h->If->Id, tmp); break; case DHCP_OPT_T1_VALUE: /* Read T1 - DHCP Renewal time */ + if (buf[index+1] != 4U) { + break; + } ctrl->t1 = get_u32 (&buf[index+2]); DEBUGF (DHCP," T1 timeout: %d sec\n",ctrl->t1); EvrNetDHCP_ViewTimeT1 (h->If->Id, ctrl->t1); @@ -1140,6 +1207,9 @@ static uint32_t proc_opt_req (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t inde case DHCP_OPT_T2_VALUE: /* Read T2 - DHCP Rebinding time */ + if (buf[index+1] != 4U) { + break; + } ctrl->t2 = get_u32 (&buf[index+2]); DEBUGF (DHCP," T2 timeout: %d sec\n",ctrl->t2); EvrNetDHCP_ViewTimeT2 (h->If->Id, ctrl->t2); @@ -1148,17 +1218,22 @@ static uint32_t proc_opt_req (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t inde case DHCP_OPT_BOOTFILE: /* Read Bootfile Name and provide it to */ /* the user as a null-terminated string. */ - tmp = buf[index + buf[index+1] + 2]; - (__CONST_CAST(uint8_t *)buf)[index + buf[index+1] + 2] = 0; - DEBUGF (DHCP," Boot fname: %s\n",&buf[index+2]); - EvrNetDHCP_ViewBootfileName (&buf[index+2], buf[index+1]+1); + dbuf = (char *)net_mem_alloc (buf[index+1] + 1); + net_strncpy (dbuf, (const char *)&buf[index+2], buf[index+1]); + /* The bootfile name is null-terminated by net_strncpy() */ + DEBUGF (DHCP," Boot fname: %s\n",dbuf); + EvrNetDHCP_ViewBootfileName (dbuf, buf[index+1] + 1); netDHCP_Notify (h->If->Id, - NET_DHCP_OPTION_BOOTFILE_NAME, &buf[index+2], buf[index+1]); - (__CONST_CAST(uint8_t *)buf)[index + buf[index+1] + 2] = tmp & 0xFF; + NET_DHCP_OPTION_BOOTFILE_NAME, (uint8_t *)dbuf, buf[index+1]); + net_mem_free (__FRAME(dbuf)); break; case DHCP_OPT_NTP_SERVER: /* Read a list of NTP Servers and provide it to the user */ + if (buf[index+1] < 4U || (buf[index+1] & 3U) != 0U) { + /* Option length must be a non-zero multiple of 4 */ + break; + } DEBUG_INFO (&buf[index]); EvrNetDHCP_ViewNtpServerList (h->If->Id, &buf[index+2], buf[index+1]); netDHCP_Notify (h->If->Id, @@ -1167,10 +1242,6 @@ static uint32_t proc_opt_req (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t inde case DHCP_OPT_PAD: return (index + 1); - - case DHCP_OPT_END: - /* End of options */ - return (0xFFFFFFFF); } /* Calculate index of next option */ index += buf[index+1] + 2; @@ -1178,7 +1249,23 @@ static uint32_t proc_opt_req (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t inde } /** - \brief Process DHCP option in state RENEWING. + \brief Parse overloaded options in RENEWING state. + \param[in] h DHCP instance handle. + \param[in] buf buffer containing DHCP options. + \param[in] start index of the first option to parse. + \param[in] end end position of the options. +*/ +static void parse_renew_opts (NET_DHCP_CFG *h, + const uint8_t *buf, uint32_t start, uint32_t end) { + uint32_t index = validate_index (start, buf, end); + while (index < end) { + index = proc_opt_renew (h, buf, index); + index = validate_index (index, buf, end); + } +} + +/** + \brief Process DHCP option in RENEWING state. \param[in] h DHCP instance handle. \param[in] buf buffer containing DHCP options. \param[in] index position of an option to process. @@ -1189,17 +1276,23 @@ static uint32_t proc_opt_renew (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t in switch (buf[index]) { case DHCP_OPT_LEASE_TIME: + if (buf[index+1] != 4U) { + break; + } tmp = get_u32 (&buf[index+2]); /* Time calculation from RFC! These values */ /* can also be received directly in options. */ - ctrl->t1 = 5 * tmp / 10; /* 0.5*tmp */ - ctrl->t2 = 7 * tmp / 8; /* 0.875*tmp */ + ctrl->t1 = tmp / 2U; /* T1 = 0.5*tmp */ + ctrl->t2 = tmp - tmp / 8U; /* T2 = 0.875*tmp */ DEBUGF (DHCP," Lease time: %d sec\n",tmp); EvrNetDHCP_ViewLeaseTime (h->If->Id, tmp); break; case DHCP_OPT_T1_VALUE: /* Read T1 - DHCP Renewal time */ + if (buf[index+1] != 4U) { + break; + } ctrl->t1 = get_u32 (&buf[index+2]); DEBUGF (DHCP," T1 timeout: %d sec\n",ctrl->t1); EvrNetDHCP_ViewTimeT1 (h->If->Id, ctrl->t1); @@ -1207,6 +1300,9 @@ static uint32_t proc_opt_renew (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t in case DHCP_OPT_T2_VALUE: /* Read T2 - DHCP Rebinding time */ + if (buf[index+1] != 4U) { + break; + } ctrl->t2 = get_u32 (&buf[index+2]); DEBUGF (DHCP," T2 timeout: %d sec\n",ctrl->t2); EvrNetDHCP_ViewTimeT2 (h->If->Id, ctrl->t2); @@ -1214,6 +1310,10 @@ static uint32_t proc_opt_renew (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t in case DHCP_OPT_ROUTER: /* Update Default Gateway IP address */ + if (buf[index+1] < 4U || (buf[index+1] & 3U) != 0U) { + /* Option length must be a non-zero multiple of 4 */ + break; + } net_addr4_copy (LocM.DefGW, &buf[index+2]); DEBUG_INF2 ("Gateway",LocM.DefGW); EvrNetDHCP_ViewGatewayAddress (h->If->Id, LocM.DefGW); @@ -1221,6 +1321,10 @@ static uint32_t proc_opt_renew (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t in case DHCP_OPT_DNS_SERVER: /* Update Primary and Secondary DNS Server */ + if (buf[index+1] < 4U || (buf[index+1] & 3U) != 0U) { + /* Option length must be a non-zero multiple of 4 */ + break; + } net_addr4_copy (LocM.PriDNS, &buf[index+2]); net_addr4_copy (LocM.SecDNS, (buf[index+1] < 8) ? net_addr_unspec : &buf[index+6]); DEBUG_INF2 ("Prim. DNS",LocM.PriDNS); @@ -1230,10 +1334,6 @@ static uint32_t proc_opt_renew (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t in case DHCP_OPT_PAD: return (index + 1); - - case DHCP_OPT_END: - /* End of options */ - return (0xFFFFFFFF); } /* Calculate index of next option */ @@ -1241,6 +1341,37 @@ static uint32_t proc_opt_renew (NET_DHCP_CFG *h, const uint8_t *buf, uint32_t in return (index); } +/** + \brief Validate an index used when parsing DHCP options. + \param[in] index index into the buffer to validate. + \param[in] buf buffer containing DHCP option data. + \param[in] len length of the buffer in bytes. + \return 'index' if the option is valid, otherwise 'len'. +*/ +static uint32_t validate_index (uint32_t index, const uint8_t *buf, uint32_t len) { + uint32_t rem; + + if (index >= len) { + return (len); + } + /* PAD and END are single-byte options */ + if (buf[index] == DHCP_OPT_PAD) { + return (index); + } + if (buf[index] == DHCP_OPT_END) { + return (len); + } + /* TLV option requires a type and length byte */ + rem = len - index; + if (rem < 2U) { + return (len); + } + if (2U + (uint32_t)buf[index + 1U] > rem) { + return (len); + } + return (index); +} + /** \brief Assign Auto-IP address. \param[in] h DHCP instance handle. diff --git a/Components/Network/Source/net_dhcp_client.h b/Components/Network/Source/net_dhcp_client.h index 2059a89e..62d0ce7a 100644 --- a/Components/Network/Source/net_dhcp_client.h +++ b/Components/Network/Source/net_dhcp_client.h @@ -1,6 +1,6 @@ /*------------------------------------------------------------------------------ * MDK Middleware - Component ::Network - * Copyright (c) 2004-2024 Arm Limited (or its affiliates). All rights reserved. + * Copyright (c) 2004-2026 Arm Limited (or its affiliates). All rights reserved. *------------------------------------------------------------------------------ * Name: net_dhcp_client.h * Purpose: Dynamic Host Configuration Client Definitions @@ -22,6 +22,12 @@ #define BOOTP_FLAG_BCAST 0x8000 // Flags - broadcast message #define BOOTP_MAGIC_COOKIE 0x63825363 // Magic Cookie +/* BOOTP fields used for DHCP option overload */ +#define BOOTP_FILE_OFFS 108U +#define BOOTP_FILE_END (BOOTP_FILE_OFFS + 128U) +#define BOOTP_SNAME_OFFS 44U +#define BOOTP_SNAME_END (BOOTP_SNAME_OFFS + 64U) + /* DHCP states */ #define DHCP_STATE_DISABLED 0 // DHCP Client is disabled #define DHCP_STATE_INIT_REBOOT 1 // DHCP Reboot, request existing IP diff --git a/Components/Network/Source/net_evr.h b/Components/Network/Source/net_evr.h index 233d2a0a..7f68a0e4 100644 --- a/Components/Network/Source/net_evr.h +++ b/Components/Network/Source/net_evr.h @@ -5658,7 +5658,7 @@ typedef struct evr_addr { \param length length of the bootfile name string */ #ifdef Network_Debug_EVR - __STATIC_INLINE void EvrNetDHCP_ViewBootfileName(const uint8_t *bootfile, uint32_t length) { + __STATIC_INLINE void EvrNetDHCP_ViewBootfileName(const char *bootfile, uint32_t length) { if (length > 32) length = 32; EventRecordData (EvtNetDHCP_ViewBootfileName, bootfile, length); } diff --git a/Documentation/Doxygen/Network/src/net_evr.txt b/Documentation/Doxygen/Network/src/net_evr.txt index 8b2e52c7..da1d0362 100644 --- a/Documentation/Doxygen/Network/src/net_evr.txt +++ b/Documentation/Doxygen/Network/src/net_evr.txt @@ -4491,7 +4491,7 @@ the lease rebinding time for the assigned IP address. */ /** -\fn __STATIC_INLINE void EvrNetDHCP_ViewBootfileName(const uint8_t *bootfile, uint32_t length) +\fn __STATIC_INLINE void EvrNetDHCP_ViewBootfileName(const char *bootfile, uint32_t length) \details The event \b ViewBootfileName is created when the network library wants to display the name of the boot file that was requested in DHCP options. From 62ad1eb30189ad95ce0c4b5c18b1bd4fcb158caa Mon Sep 17 00:00:00 2001 From: Franc Urbanc Date: Thu, 1 Oct 2026 10:14:30 +0200 Subject: [PATCH 2/5] Network: fix TFTP server filename being treated as an unbounded C string --- Components/Network/Source/net_tftp_server.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Components/Network/Source/net_tftp_server.c b/Components/Network/Source/net_tftp_server.c index 7d17620b..a99dca22 100644 --- a/Components/Network/Source/net_tftp_server.c +++ b/Components/Network/Source/net_tftp_server.c @@ -1,6 +1,6 @@ /*------------------------------------------------------------------------------ * MDK Middleware - Component ::Network - * Copyright (c) 2004-2025 Arm Limited (or its affiliates). All rights reserved. + * Copyright (c) 2004-2026 Arm Limited (or its affiliates). All rights reserved. *------------------------------------------------------------------------------ * Name: net_tftp_server.c * Purpose: Trivial File Transfer Protocol Server @@ -453,6 +453,13 @@ static uint32_t tftp_listener (int32_t socket, const NET_ADDR *addr, buf += 2; len -= 2; + + /* Check if filename is null-terminated and followed by the mode field */ + if (!get_next_opt (buf, buf + len)) { + tftp_abort_session (tftp_s, TFTP_ERR_ILLOPCOD, "Illegal operation"); + return (false); + } + DEBUGF (TFTP," File : %s\n",buf); EvrNetTFTPs_FileRequested ((const char *)buf, strlen((const char *)buf)); From 798b01f10771b48d17dbd9ee28804d33a7c37ede Mon Sep 17 00:00:00 2001 From: Franc Urbanc Date: Thu, 1 Oct 2026 09:51:16 +0200 Subject: [PATCH 3/5] Network: fix potential out-of-bounds reads in FTP client --- Components/Network/Source/net_ftp_client.c | 77 ++++++++++++++-------- 1 file changed, 51 insertions(+), 26 deletions(-) diff --git a/Components/Network/Source/net_ftp_client.c b/Components/Network/Source/net_ftp_client.c index 14353f02..8f48beaf 100644 --- a/Components/Network/Source/net_ftp_client.c +++ b/Components/Network/Source/net_ftp_client.c @@ -1,6 +1,6 @@ /*------------------------------------------------------------------------------ * MDK Middleware - Component ::Network - * Copyright (c) 2004-2025 Arm Limited (or its affiliates). All rights reserved. + * Copyright (c) 2004-2026 Arm Limited (or its affiliates). All rights reserved. *------------------------------------------------------------------------------ * Name: net_ftp_client.c * Purpose: File Transfer Client @@ -26,7 +26,7 @@ static uint32_t ftpc_listener (int32_t socket, netTCP_Event event, const NET_ADD static uint32_t ftpc_data_cb (int32_t socket, netTCP_Event event, const NET_ADDR *addr, const uint8_t *buf, uint32_t len); static netStatus ftpc_dopen_req (void); -static uint16_t ftpc_scan_dport (const char *buf); +static uint16_t ftpc_scan_dport (const char *buf, uint32_t len); static void ftpc_close_file (void); static void ftpc_transit (uint8_t state); static void ftpc_stop (netFTPc_Event event); @@ -290,7 +290,7 @@ dopen: ftpc_transit ((ftpc_s->Flags & FTPC_FLAG_PASSIVE) ? (ftpc_s->Resp != FTPC_RESP_EPSVOK)) { break; } - ftpc_s->DPort = ftpc_scan_dport ((const char *)buf); + ftpc_s->DPort = ftpc_scan_dport ((const char *)buf, len); if (ftpc_s->DPort == 0) { ERRORF (FTPC,"PASV command failed\n"); EvrNetFTPc_PasvCommandFailed (); @@ -960,35 +960,59 @@ static netStatus ftpc_dopen_req (void) { /** \brief Scan PASV/EPSV response for FTP data port number. \param[in] buf buffer containing response parameters. + \param[in] len length of the response. \return data port number or 0=not found. */ -static uint16_t ftpc_scan_dport (const char *buf) { - int32_t max,m,n,port; +static uint16_t ftpc_scan_dport (const char *buf, uint32_t len) { + int32_t i,start,d,n,port; + start = net_strnchr (buf, '(', len); + if (start < 0) { + return (0); + } + if (net_strnchr (buf+start, ')', len-start) < 6) { + return (0); + } + + buf += (start + 1); if (ftpc_s->Resp == FTPC_RESP_PASVOK) { /* PASV example: (192,168,0,162,19,137) */ - for (n = 0, max = 60; max; buf++, max--) { - if (*buf == ',') { - if (++n == 4) { - port = net_atoi (++buf, &m) << 8; - port |= net_atoi (++buf+m, NULL); - return (port & 0xFFFF); - } + /* buf: 192,168,0,162,19,137) */ + for (i = 0; i < 4; i++) { + d = net_atoi (buf, &n); + if (buf[n] != ',' || d < 0 || d > 255) { + return (0); } + buf += (n + 1); } + /* buf: 19,137) */ + d = net_atoi (buf, &n); + if (buf[n] != ',' || d < 0 || d > 255) { + return (0); + } + buf += (n + 1); + port = d << 8; + d = net_atoi (buf, &n); + if (buf[n] != ')' || d < 0 || d > 255) { + return (0); + } + port |= d; } else { /* EPSV example: (|||5282|) */ - for (n = 0, max = 60; max; buf++, max--) { - if (*buf == '|') { - if (++n == 3) { - port = net_atoi (++buf, NULL); - return (port & 0xFFFF); - } - } + /* buf: |||5282|) */ + char delim = buf[0]; + if ((buf[1] != delim) || (buf[2] != delim)) { + return (0); + } + buf += 3; + /* buf: 5282|) */ + port = net_atoi (buf, &n); + if (buf[n] != delim || port < 0 || port > 65535) { + return (0); } } - return (0); + return (port & 0xFFFF); } /** @@ -1046,28 +1070,29 @@ static bool ftpc_proc_resp (const char *buf, const char *end) { ftpc_s->Flags |= FTPC_FLAG_FIRST; ftpc_s->Resp = get_resp (buf); buf += 3; - if ((buf[0] != '-') || (buf >= end)) { + if ((buf >= end) || (buf[0] != '-')) { /* This is a single line response, we are done */ return (true); } /* Get next line */ - for ( ; (buf[0] != '\n') && (buf < end); buf++); + for ( ; (buf < end) && (buf[0] != '\n'); buf++); if (++buf >= end) { return (false); } } /* Process multi-line response */ - for (;;) { + for ( ; buf+4 <= end; ) { if ((get_resp (buf) == ftpc_s->Resp) && (buf[3] != '-')) { /* Terminating reply code */ - break; + return (true); } - for ( ; (buf[0] != '\n') && (buf < end); buf++); + /* Get next line */ + for ( ; (buf < end) && (buf[0] != '\n'); buf++); if (++buf >= end) { return (false); } } - return (true); + return (false); } /** From 9b665e0fb0f1a38b72847fc125d14911ef098905 Mon Sep 17 00:00:00 2001 From: Franc Urbanc Date: Thu, 1 Oct 2026 10:57:05 +0200 Subject: [PATCH 4/5] Network: fix SMTP client unsigned underflow and Base64 over-read/write --- Components/Network/Source/net_base64.c | 21 ++++++++++--------- .../Network/Source/net_smtp_client_core.c | 16 +++++++------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/Components/Network/Source/net_base64.c b/Components/Network/Source/net_base64.c index a453ff94..bc14b901 100644 --- a/Components/Network/Source/net_base64.c +++ b/Components/Network/Source/net_base64.c @@ -1,6 +1,6 @@ /*------------------------------------------------------------------------------ * MDK Middleware - Component ::Network - * Copyright (c) 2004-2024 Arm Limited (or its affiliates). All rights reserved. + * Copyright (c) 2004-2026 Arm Limited (or its affiliates). All rights reserved. *------------------------------------------------------------------------------ * Name: net_base64.c * Purpose: Base64 Encode and Decode @@ -61,14 +61,15 @@ uint32_t net_base64_encode (char *base64, const void *buf, uint32_t len) { } /** - \brief Decode base64 encoded string to binary data. + \brief Decode base64 encoded string into binary data. \param[out] buf output buffer to store the data. - \param[in] base64 pointer to encoded input string. + \param[in] base64 pointer to base64 encoded input string. \param[in] len maximum length of the input string. \return length of converted binary data. */ uint32_t net_base64_decode (void *buf, const char *base64, uint32_t len) { - /* Base64 decoding table: characters 43 to 122 ('+' to 'z') */ + /* Base64 decoding table: characters from '+' (43) through 'z' (122) */ + /* Values 0..63 are valid base64 values; 0xFF marks invalid characters */ static const uint8_t dec_map[80] = { "\x3E\xFF\xFF\xFF\x3F\x34\x35\x36\x37\x38\x39\x3A\x3B\x3C\x3D\xFF" "\xFF\xFF\xFF\xFF\xFF\xFF\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09" @@ -79,28 +80,28 @@ uint32_t net_base64_decode (void *buf, const char *base64, uint32_t len) { uint8_t *dp = buf; uint32_t i,j,v,idx; - for (i = 0; len > 0; base64 += 4, i += 3) { - /* Convert 4 base64 characters to 3 binary bytes */ + for (i = 0; len > 3; base64 += 4, len -= 4, i += 3) { + /* Convert four 6-bit base64 values into 24-bit output value */ for (v = j = 0; j < 4; j++) { if (j == 2 && base64[j] == '=' && base64[j+1] == '=') { - /* Padding, decode last byte */ + /* '==' padding, only one output byte remains */ dp[i] = (v >> 4) & 0xFF; return (i + 1); } if (j == 3 && base64[j] == '=') { - /* Padding, decode last 2 bytes */ + /* '=' padding, two output bytes remain */ dp[i] = (v >> 10) & 0xFF; dp[i+1] = (v >> 2) & 0xFF; return (i + 2); } idx = base64[j] - 43; if (idx > 79 || dec_map[idx] > 63) { - /* End, invalid character found*/ + /* Stop decoding, invalid base64 character found */ return (i); } v = v << 6 | dec_map[idx]; - len--; } + /* Split 24-bit value into three decoded bytes */ dp[i] = (v >> 16) & 0xFF; dp[i+1] = (v >> 8) & 0xFF; dp[i+2] = v & 0xFF; diff --git a/Components/Network/Source/net_smtp_client_core.c b/Components/Network/Source/net_smtp_client_core.c index c435b188..fba7795a 100644 --- a/Components/Network/Source/net_smtp_client_core.c +++ b/Components/Network/Source/net_smtp_client_core.c @@ -192,10 +192,10 @@ static uint32_t smtp_listener (int32_t socket, netTCP_Event event, const NET_ADD return (false); } /* Get reply from server */ - if(len < 3) { - /* Data frame too short */ + if (len < 5) { + /* Minimum SMTP response is 3-digit reply code + CRLF */ ERRORF (SMTP,"Process, Frame too short\n"); - EvrNetSMTP_FrameTooShort (len, 3); + EvrNetSMTP_FrameTooShort (len, 5); return (false); } @@ -303,9 +303,8 @@ static uint32_t smtp_listener (int32_t socket, netTCP_Event event, const NET_ADD (__CONST_CAST(char *)buf)[len-1] = 0; DEBUGF (SMTP," Prompt: %s\n",buf+4); #endif - /* Decode base64 string to the same buffer */ - net_base64_decode (__CONST_CAST(char *)buf+4, (const char *)buf+4, len-4); - if (net_strcmp ((const char *)buf+4, "Username:")) { + /* Base64-encoded "Username:" prompt for AUTH LOGIN */ + if ((len > 16) && net_strcmp ((const char *)buf+4, "VXNlcm5hbWU6")) { smtp_transit (SMTP_STATE_USER); return (true); } @@ -347,9 +346,8 @@ static uint32_t smtp_listener (int32_t socket, netTCP_Event event, const NET_ADD (__CONST_CAST(char *)buf)[len-1] = 0; DEBUGF (SMTP," Prompt: %s\n",buf+4); #endif - /* Decode base64 string to the same buffer */ - net_base64_decode (__CONST_CAST(char *)buf+4, (const char *)buf+4, len-4); - if (net_strcmp ((const char *)buf+4, "Password:")) { + /* Base64-encoded "Password:" prompt for AUTH LOGIN */ + if ((len > 16) && net_strcmp ((const char *)buf+4, "UGFzc3dvcmQ6")) { smtp->Flags |= SMTP_FLAG_PASSW; smtp_transit (SMTP_STATE_USER); return (true); From 9f6efc187c6e9b03d3f20cb450eae836f452a5db Mon Sep 17 00:00:00 2001 From: Franc Urbanc Date: Fri, 16 Oct 2026 11:53:23 +0200 Subject: [PATCH 5/5] Network: update version and revision history --- Documentation/Doxygen/Network/src/revision_history.md | 4 ++++ Keil.MDK-Middleware.pdsc | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/Documentation/Doxygen/Network/src/revision_history.md b/Documentation/Doxygen/Network/src/revision_history.md index 07760632..dd667615 100644 --- a/Documentation/Doxygen/Network/src/revision_history.md +++ b/Documentation/Doxygen/Network/src/revision_history.md @@ -8,6 +8,10 @@ V8.3.1 + - fixed SMTP client unsigned underflow and Base64 over-read/write + - fixed potential out-of-bounds reads in FTP client + - fixed TFTP server filename being treated as an unbounded C string + - fixed parsing of malformed DHCP options - fixed possible TCP socket reset after timeout recovery - various minor fixes and security improvements diff --git a/Keil.MDK-Middleware.pdsc b/Keil.MDK-Middleware.pdsc index e867369e..4f0aab2e 100644 --- a/Keil.MDK-Middleware.pdsc +++ b/Keil.MDK-Middleware.pdsc @@ -23,6 +23,10 @@ - corrected FAT name cache entry size from 48 to 40 bytes - corrected various minor code quality issues Network Component Version 8.3.1 + - fixed SMTP client unsigned underflow and Base64 over-read/write + - fixed potential out-of-bounds reads in FTP client + - fixed TFTP server filename being treated as an unbounded C string + - fixed parsing of malformed DHCP options - fixed possible TCP socket reset after timeout recovery - various minor fixes and security improvements USB Component Version 8.0.3