Date: Sun, 13 Apr 2003 14:20:39 -0400 (EDT) From: User & <barney@lab.databus.com> To: FreeBSD-gnats-submit@FreeBSD.org Subject: bin/50909: ping checks too much data in return packet Message-ID: <200304131820.h3DIKdhv039109@lab.databus.com> Resent-Message-ID: <200304131830.h3DIUABN074632@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 50909
>Category: bin
>Synopsis: ping checks too much data in return packet
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Sun Apr 13 11:30:10 PDT 2003
>Closed-Date:
>Last-Modified:
>Originator: Barney Wolff
>Release: FreeBSD 5.0-CURRENT i386
>Organization:
Databus Inc.
>Environment:
System: FreeBSD lab.databus.com 5.0-CURRENT FreeBSD 5.0-CURRENT #1: Fri Apr 11 18:00:56 EDT 2003 toor@lab.databus.com:/usr/obj/usr/src/sys/LAB i386
>Description:
ping checks the data in the returned packet, expecting it to be equal
to the data in the packet sent. But it starts the check too early,
checking the timestamp. If the returned packet comes back after the
next packet has already been sent (ie, in the default case, after 1 sec)
the check will fail. ping should check only the constant data, which
starts after the timestamp.
I'd also point out that the cc != 0 check is bad style, if probably
not dangerous in this case. cc > 0 protects against off-by-one errors.
cc-- is also bad style.
>How-To-Repeat:
ping anywhere with rtt over 1 sec.
>Fix:
Index: ping.c
===================================================================
RCS file: /home/ncvs/src/sbin/ping/ping.c,v
retrieving revision 1.95
diff -u -r1.95 ping.c
--- ping.c 7 Apr 2003 12:05:50 -0000 1.95
+++ ping.c 13 Apr 2003 18:07:02 -0000
@@ -1012,8 +1012,14 @@
cp = (u_char*)&icp->icmp_data[phdr_len];
dp = &outpack[MINICMPLEN + phdr_len];
cc -= ICMP_MINLEN + phdr_len;
- for (i = phdr_len; i < datalen && cc != 0;
- ++i, ++cp, ++dp, cc--) {
+ i = phdr_len;
+ if (timing) { /* don't check variable timestamp */
+ cp += TIMEVAL_LEN;
+ dp += TIMEVAL_LEN;
+ cc -= TIMEVAL_LEN;
+ i += TIMEVAL_LEN;
+ }
+ for ( ; i < datalen && cc > 0; ++i, ++cp, ++dp, --cc) {
if (*cp != *dp) {
(void)printf("\nwrong data byte #%d should be 0x%x but was 0x%x",
i, *dp, *cp);
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200304131820.h3DIKdhv039109>