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: iputils#542 Reported-by: mimicria Suggested-by: Marius Tomaschewski <[email protected]> Signed-off-by: Petr Vorel <[email protected]>
pevik · Aug 16, 2024 · 37046b2 · 37046b2
1 parent 0f12e6d
commit 37046b2
Show file tree

Hide file tree

Showing 4 changed files with 5 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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)