[PATCH v2 1/2] net: bootp: validate DHCP option length before parsing it
Pranav Rajendran <[email protected]>
| Newsgroups | org.u-boot-project.lists.u-boot |
|---|---|
| Message-ID | <[email protected]> |
dhcp_process_options() reads the option length byte and dereferences
the option payload without checking either is inside [popt, end):
while (popt < end && *popt != 0xff) {
oplen = *(popt + 1);
switch (*popt) {
case 0:
oplen = -1; /* Pad omits len byte */
break;
case 1:
net_copy_ip(&net_netmask, (popt + 2));
...
The loop guard only proves *popt is readable. If a packet ends right
after an option code byte, popt + 1 is already one past the received
data, so oplen = *(popt + 1) reads out of bounds. The pad case (0)
hits this unconditionally, since oplen is read before the switch
even determines the option is a pad.
Once oplen is read, nothing checks that popt + 2 + oplen - the option
header plus its declared payload - is still within end before the
switch dereferences popt + 2 onward (net_copy_ip, memcpy, strlcpy,
the option-52 overload byte, and the PXE config file allocation all
do this). A short final option with an oplen that overruns the buffer
is processed as if the payload were present, so out-of-bounds bytes
are copied into net_netmask, net_root_path, dhcp_option_overload, and
similar globals that go on to influence boot behaviour.
Handle the pad option before touching a second byte, require a length
byte to exist before reading it, and require the full declared option
(header + payload) to fit before entering the switch. A truncated
trailing option now stops parsing instead of reading past the buffer.
This is a prerequisite for bounding dhcp_process_options() by the
received packet length rather than by BOOTP_HDR_SIZE: fixing the outer
limit alone leaves this inner out-of-bounds read reachable whenever a
short reply's last option is cut off before its length or payload
bytes.
Signed-off-by: Pranav Rajendran <[email protected]>
---
v2: New patch, added in response to review feedback on v1 of
"net: bootp: bound DHCP option parsing by the received packet
length" pointing out this inner gap.
net/bootp.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/net/bootp.c b/net/bootp.c
index f0dc329d6e4..eafbe9e3bb4 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -863,11 +863,20 @@ static void dhcp_process_options(uchar *popt, uchar *end)
#endif
while (popt < end && *popt != 0xff) {
+ if (*popt == 0) {
+ /* Pad option: single byte, no length field */
+ popt++;
+ continue;
+ }
+
+ /* Need a length byte, and the payload it describes */
+ if (popt + 1 >= end)
+ break;
oplen = *(popt + 1);
- switch (*popt) {
- case 0:
- oplen = -1; /* Pad omits len byte */
+ if (popt + 2 + oplen > end)
break;
+
+ switch (*popt) {
case 1:
net_copy_ip(&net_netmask, (popt + 2));
break;
--
2.50.1 (Apple Git-155)