[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [PATCH] udhcp: Avoid leaking uninitialized/stale data
From:       Denys Vlasenko <vda.linux () googlemail ! com>
Date:       2023-10-04 14:21:37
Message-ID: CAK1hOcNaZ6gV3wkcSK-X8MVTACeH2Z-MkAEFXFQCEVzKoCVdvQ () mail ! gmail ! com
[Download RAW message or body]

Applied, thank you

On Mon, Oct 2, 2023 at 9:34 PM Russ Dill <russ.dill@gmail.com> wrote:
> 
> 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
>  
> Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
> Cc: Colin Wee <cwee@tesla.com>
> Cc: Denys Vlasenko <vda.linux@googlemail.com>
> ---
> networking/udhcp/d6_dhcpc.c | 1 +
> networking/udhcp/dhcpc.c    | 1 +
> 2 files changed, 2 insertions(+)
> 
> diff --git a/networking/udhcp/d6_dhcpc.c b/networking/udhcp/d6_dhcpc.c
> index cdd06188e..a72fd31bd 100644
> --- a/networking/udhcp/d6_dhcpc.c
> +++ b/networking/udhcp/d6_dhcpc.c
> @@ -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;
> }
> diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c
> index 200a2fb8a..fc86b1607 100644
> --- a/networking/udhcp/dhcpc.c
> +++ b/networking/udhcp/dhcpc.c
> @@ -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;
> }
> --
> 2.40.1
> 
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


[prev in list] [next in list] [prev in thread] [next in thread]