Subject: bin/36997: ping doesn't verify limits
To: None <gnats-admin@netbsd.org, netbsd-bugs@netbsd.org>
From: None <zafer@aydogan.de>
List: netbsd-bugs
Date: 09/17/2007 10:15:00
>Number:         36997
>Category:       bin
>Synopsis:       ping doesn't verify limits
>Confidential:   no
>Severity:       non-critical
>Priority:       high
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Sep 17 10:15:00 +0000 2007
>Originator:     Zafer Aydogan
>Release:        4.99.31
>Organization:
>Environment:
i386
>Description:
I think I found a bug in ping.

When playing around with ping, I noticed that there is no sufficient
verification for limits for the options -i, -l and -w.
For example, if you enter a really big interval, then poll() bails
out, since it can only handle  LONG_MAX (2147483647). But entering far
larger number lets ping hang.
More interesting and weird is that, if the interval is smaller than
0.000001 then ping hangs aswell. I thought everything less than
0.000001 should be treated like 0, but it isn't.

In my patch I've split the error messages to be more friendly and precise.
I multiply interval with 1000 to match LONG_MAX, which means the
largest interval will be 2147482 and the smallest will be 0.0001.
Smaller ones will be treated like 0.0001 (1000 Hz).

My patch may be incomplete or wrong.
Please review it at: http://www.aydogan.org/ping/ping.diff
(written against current).

When verified, a patch should be pulled up to 4.0 aswell.

Thanks, Zafer.
>How-To-Repeat:
ping -i 0.0000001 127.0.0.1
ping -i 9999999999 127.0.0.1

and
 
ping -i 99999999 127.0.0.1
>Fix:
--- ping.c.orig	2007-09-15 16:11:39.000000000 +0200
+++ ping.c	2007-09-15 18:36:32.000000000 +0200
@@ -283,8 +283,11 @@
 			break;
 		case 'c':
 			npackets = strtol(optarg, &p, 0);
-			if (*p != '\0' || npackets <= 0)
-				errx(1, "Bad/invalid number of packets");
+			if (*p != '\0') 
+				errx(1, "Invalid number of packets");
+
+			if (npackets <= 0 || npackets > INT_MAX)
+				errx(1, "Bad number of packets");
 			break;
 		case 'D':
 			pingflags |= F_DF;
@@ -300,13 +303,23 @@
 			break;
 		case 'i':		/* wait between sending packets */
 			interval = strtod(optarg, &p);
-			if (*p != '\0' || interval <= 0)
-				errx(1, "Bad/invalid interval %s", optarg);
+			if (*p != '\0')
+				errx(1, "Invalid timing interval %s", optarg);
+
+			if (interval <= 0 || interval * 1000 > INT_MAX)
+				errx(1, "Bad timing interval %s", optarg);
+			
+			if (interval < 0.0001) 
+				interval = 0.0001;
 			break;
 		case 'l':
 			preload = strtol(optarg, &p, 0);
-			if (*p != '\0' || preload < 0)
-				errx(1, "Bad/invalid preload value %s",
+			if (*p != '\0')
+				errx(1, "Invalid preload value %s",
+				     optarg);
+
+			if (preload < 0 || preload > INT_MAX)
+				errx(1, "Bad preload value %s",
 				     optarg);
 			break;
 		case 'n':
@@ -372,8 +385,11 @@
 			break;
 		case 'w':
 			maxwait = strtod(optarg, &p);
-			if (*p != '\0' || maxwait <= 0)
-				errx(1, "Bad/invalid maxwait time %s", optarg);
+			if (*p != '\0') 
+				errx(1, "Invalid maxwait time %s", optarg);
+
+			if ( maxwait <= 0 || maxwait > INT_MAX)
+				errx(1, "Bad maxwait time %s", optarg);
 			break;
 #ifdef IPSEC
 #ifdef IPSEC_POLICY_IPSEC