[PATCH] udpsvd: do not copy more of the listen address than was allocated
Ali Ahmet Memis via busybox <[email protected]>
| Newsgroups | gmane.linux.busybox |
|---|---|
| Message-ID | <[email protected]> |
len_and_sockaddr is allocated for the address family it holds, so for
an IPv4 address str2sockaddr() allocates LSA_LEN_SIZE plus
sizeof(struct sockaddr_in), which is 20 bytes. The type itself is
larger when IPv6 support is enabled, because the union then contains
struct sockaddr_in6, so sizeof(len_and_sockaddr) is 32.
The udp path assigns the whole structure:
local = *lsa;
That copies 32 bytes out of a 20 byte allocation and reads 12 bytes
past its end. AddressSanitizer reports it on every datagram:
READ of size 32 at ...
#0 tcpudpsvd_main networking/tcpudp.c:423
0x... is located 0 bytes after 20-byte region
#0 str2sockaddr libbb/xconnect.c:231
The bytes end up in the part of the union that an IPv4 socket never
looks at, so nothing is printed or sent, but the read itself is out of
bounds. Copy sa_len bytes of the address instead; local.len is already
set on the line above.
text data bss dec hex filename
2884 0 0 2884 b44 networking/tcpudp.o before
2874 0 0 2874 b3a networking/tcpudp.o after
-10
Signed-off-by: Ali Ahmet Memis <[email protected]>
---
I found this while testing the tftpd patch I sent earlier. udpsvd
was the service running in front of it.
The recent patches are spread across different files because I've
been looking at parsers handling untrusted input rather than focusing
on a single applet.
networking/tcpudp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/networking/tcpudp.c b/networking/tcpudp.c
index 708e05c2e..45d0c23bf 100644
--- a/networking/tcpudp.c
+++ b/networking/tcpudp.c
@@ -420,7 +420,8 @@ int tcpudpsvd_main(int argc UNUSED_PARAM, char **argv)
} else {
/* In case recv_from_to won't be able to recover local addr.
* Also sets port - recv_from_to is unable to do it. */
- local = *lsa;
+ /* lsa is allocated for its address family, copy only that much */
+ memcpy(&local.u.sa, &lsa->u.sa, sa_len);
conn = recv_from_to(sock, NULL, 0, MSG_PEEK,
&remote.u.sa, &local.u.sa, sa_len);
}
--
2.55.0