From 37046b20a3635ab35bd7f96afb3d1b270e82475d Mon Sep 17 00:00:00 2001 From: Petr Vorel Date: Fri, 16 Aug 2024 10:59:25 +0200 Subject: [PATCH] ping: Fix integer overflow for high -s values a647e2c commit increased value max allowed -s value from 127992 to INT_MAX (2147483647). To have space for this range datalen was changed from signed int to size_t, but it forgot to change other int variables which also work with this data (hold, packlen). This fixes signed integer overflow: $ export CC="clang" $ export CFLAGS="-O0 -g -fsanitize=address,undefined" $ export LDFLAGS="-O0 -g -fsanitize=address,undefined" $ ./configure && make $ ./builddir/ping/ping -s 2147483647 ::1 ../ping/ping6_common.c:317:7: runtime error: signed integer overflow: -2147483641 + -1174404560 cannot be represented in type 'int' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../ping/ping6_common.c:317:7 ./builddir/ping/ping: WARNING: probably, rcvbuf is not enough to hold preload PING ::1 (::1) 2147483647 data bytes $ ./builddir/ping/ping -s 2147483647 127.0.0.1 ../ping/ping.c:997:7: runtime error: signed integer overflow: -2147483641 + -1090518520 cannot be represented in type 'int' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../ping/ping.c:997:7 ./builddir/ping/ping: WARNING: probably, rcvbuf is not enough to hold preload PING 127.0.0.1 (127.0.0.1) 2147483647(2147483675) bytes of data. NOTE: I'm not sure if it makes sense to allow -s higher than 65507 (IPv4) or 65527 (IPv6) - more than these is over limit for IPv4/IPv6 packet payload. It would have to be sent as multiple packets (IPv4 fragmentation, IPv6 next_header packet chain or jumbo extension header). IMHO it never worked. That is probably the reason, why other implementations (busybox, fping, inetutils) limit data size to <= 65535. Fixes: a647e2c ("ping: allow any package size to be defined by user") Fixes: https://github.com/iputils/iputils/issues/542 Reported-by: mimicria Suggested-by: Marius Tomaschewski Signed-off-by: Petr Vorel --- ping/ping.c | 2 +- ping/ping.h | 2 +- ping/ping6_common.c | 3 +-- ping/ping_common.c | 4 ++-- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/ping/ping.c b/ping/ping.c index 55884328..29ce3497 100644 --- a/ping/ping.c +++ b/ping/ping.c @@ -749,7 +749,7 @@ int ping4_run(struct ping_rts *rts, int argc, char **argv, struct addrinfo *ai, .ai_protocol = IPPROTO_UDP, .ai_flags = getaddrinfo_flags }; - int hold, packlen; + size_t hold, packlen; unsigned char *packet; char *target; char hnamebuf[NI_MAXHOST]; diff --git a/ping/ping.h b/ping/ping.h index 3e2e3c33..fe663f16 100644 --- a/ping/ping.h +++ b/ping/ping.h @@ -399,7 +399,7 @@ char *str_interval(int interval); int is_ours(struct ping_rts *rts, socket_st *sock, uint16_t id); extern int pinger(struct ping_rts *rts, ping_func_set_st *fset, socket_st *sock); -extern void sock_setbufs(struct ping_rts *rts, socket_st *, int alloc); +extern void sock_setbufs(struct ping_rts *rts, socket_st *sock, size_t alloc); extern void sock_setmark(struct ping_rts *rts, int fd); extern void setup(struct ping_rts *rts, socket_st *); extern int main_loop(struct ping_rts *rts, ping_func_set_st *fset, socket_st*, diff --git a/ping/ping6_common.c b/ping/ping6_common.c index eb3b0d23..ba38eaf9 100644 --- a/ping/ping6_common.c +++ b/ping/ping6_common.c @@ -104,8 +104,7 @@ unsigned int if_name2index(const char *ifname) int ping6_run(struct ping_rts *rts, int argc, char **argv, struct addrinfo *ai, struct socket_st *sock) { - int hold, packlen; - size_t i; + size_t hold, i, packlen; unsigned char *packet; char *target; struct icmp6_filter filter; diff --git a/ping/ping_common.c b/ping/ping_common.c index ebe78880..13cac782 100644 --- a/ping/ping_common.c +++ b/ping/ping_common.c @@ -437,9 +437,9 @@ int pinger(struct ping_rts *rts, ping_func_set_st *fset, socket_st *sock) /* Set socket buffers, "alloc" is an estimate of memory taken by single packet. */ -void sock_setbufs(struct ping_rts *rts, socket_st *sock, int alloc) +void sock_setbufs(struct ping_rts *rts, socket_st *sock, size_t alloc) { - int rcvbuf, hold; + size_t rcvbuf, hold; socklen_t tmplen = sizeof(hold); if (!rts->sndbuf)