BusyBox
/
busybox
mirror of https://github.com/mirror/busybox.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

udhcp: Avoid leaking uninitialized/stale data 

I noticed a commit in connman:

"gdhcp: Avoid leaking stack data via unitiialized variable" [1]

Since gdhcp is just BusyBox udhcp with the serial numbers filed off, I
checked if BusyBox udhcp has a related issue.

The issue is that the get_option logic assumes any data within the
memory area of the buffer is "valid". This reduces the complexity of the
function at the cost of reading past the end of the actually received
data in the case of specially crafted packets. This is not a problem
for the udhcp_recv_kernel_packet data path as the entire memory
area is zeroed. However, d4/d6_recv_raw_packet does not zero the
memory.

Note that a related commit [2] is not required as we are zeroing
any data that can be read by the get_option function.

[1] https://git.kernel.org/pub/scm/network/connman/connman.git/commit/?id=a74524b3e3fad81b0fd1084ffdf9f2ea469cd9b1
[2] https://git.kernel.org/pub/scm/network/connman/connman.git/commit/?id=58d397ba74873384aee449690a9070bacd5676fa

function                                             old     new   delta
d4_recv_raw_packet                                   484     497     +13
d6_recv_raw_packet                                   216     228     +12
.rodata                                           105390  105381      -9
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 2/1 up/down: 25/-9)              Total: 16 bytes

Signed-off-by: Russ Dill <russ.dill@gmail.com>
Cc: Colin Wee <cwee@tesla.com>
Cc: Denys Vlasenko <vda.linux@googlemail.com>
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
master
Russ Dill 2023-10-02 12:34:50 -07:00 committed by Denys Vlasenko
parent 5fa39d48d5
commit e265c8d4c0
2 changed files with 3 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
networking/udhcp/d6_dhcpc.c
View File

@ -961,6 +961,7 @@ static NOINLINE int d6_recv_raw_packet(struct in6_addr *peer_ipv6, struct d6_pac
d6_dump_packet(&packet.data);
bytes -= sizeof(packet.ip6) + sizeof(packet.udp);
memset(d6_pkt, 0, sizeof(*d6_pkt));
memcpy(d6_pkt, &packet.data, bytes);
return bytes;
}

3
networking/udhcp/dhcpc.c
View File

@ -966,7 +966,7 @@ static NOINLINE int d4_recv_raw_packet(struct dhcp_packet *dhcp_pkt, int fd)
check = packet.udp.check;
packet.udp.check = 0;
if (check && check != inet_cksum(&packet, bytes)) {
log1s("packet with bad UDP checksum received, ignoring");
log1s("packet with bad UDP checksum, ignoring");
return -2;
}
skip_udp_sum_check:
@ -981,6 +981,7 @@ static NOINLINE int d4_recv_raw_packet(struct dhcp_packet *dhcp_pkt, int fd)
udhcp_dump_packet(&packet.data);
bytes -= sizeof(packet.ip) + sizeof(packet.udp);
memset(dhcp_pkt, 0, sizeof(*dhcp_pkt));
memcpy(dhcp_pkt, &packet.data, bytes);
return bytes;
}