]> The Tcpdump Group git mirrors - tcpdump/commitdiff
Fix "ip broadcast" netmask byte order with the -f flag. 1299/head
authorDenis Ovsienko <[email protected]>
Sat, 22 Feb 2025 02:29:41 +0000 (02:29 +0000)
committerDenis Ovsienko <[email protected]>
Sat, 22 Feb 2025 10:58:56 +0000 (10:58 +0000)
Let's suppose the interface eth0 has one IPv4 address with a /24
netmask.  Without -f tcpdump leaves the netmask variable set to 0, which
regardless of the host endianness causes "ip broadcast" to match
destination hosts 0.0.0.0 and 255.255.255.255:

# tcpdump -i eth0 -d 'ip broadcast'
(000) ldh      [12]
(001) jeq      #0x800           jt 2 jf 6
(002) ld       [30]
(003) jeq      #0x0             jt 5 jf 4
(004) jeq      #0xffffffff      jt 5 jf 6
(005) ret      #262144
(006) ret      #0

With -f tcpdump calls pcap_lookupnet(), which correctly sets the netmask
to 0xFFFFFF00 (in network byte order).  Then pcap_compile() receives the
same value, but it expects it to be in host byte order, so on a
little-endian host the resulting filter program incorrectly tests for a
0x00FFFFFF netmask:

# tcpdump -i eth0 -f -d 'ip broadcast'
(000) ldh      [12]
(001) jeq      #0x800           jt 2 jf 7
(002) ld       [30]
(003) jset     #0xff000000      jt 4 jf 6
(004) and      #0xff000000
(005) jeq      #0xff000000      jt 6 jf 7
(006) ret      #262144
(007) ret      #0

Add two missing ntohl() wrappers to make it right:
# tcpdump -i eno1 -f -d 'ip broadcast'
(000) ldh      [12]
(001) jeq      #0x800           jt 2 jf 7
(002) ld       [30]
(003) jset     #0xff            jt 4 jf 6
(004) and      #0xff
(005) jeq      #0xff            jt 6 jf 7
(006) ret      #262144
(007) ret      #0

Audit the init_print() code path and do not change anything because
there the byte order is already correct.  Add comments to spell the byte
order in every case and update the -f flag description in the man page.
See also libpcap commit 1e54958.

CHANGES
addrtoname.c
print.c
tcpdump.1.in
tcpdump.c

diff --git a/CHANGES b/CHANGES
index 4749af81fe9fe73e43c33d83cc8f17792820039f..3dabeefea5429aec2fcac6d76c0e9172737d2cf6 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -66,6 +66,7 @@ DayOfTheWeek, Month DD, YYYY / The Tcpdump Group
       Change Sun RPC code licence to BSD-3-Clause.
       Use C99 macros to define 64-bit constants and maximum 64-bit
         values.
+      Fix "ip broadcast" netmask byte order with the -f flag.
     Building and testing:
       Autoconf: Remove detection of early IPv6 stacks.
       Detect OS IPv6 support using AF_INET6 only.
index 48fac28696d7be8368b785d9316764e80b64991a..43bfc47d82a28be0c4bbbe565949f88addfdfbe5 100644 (file)
@@ -290,6 +290,7 @@ ipaddr_string(netdissect_options *ndo, const u_char *ap)
         *      (2) Address is foreign and -f was given. (If -f was not
         *          given, f_netmask and f_localnet are 0 and the test
         *          evaluates to true)
+        * Both addr and f_netmask and f_localnet are in network byte order.
         */
        if (!ndo->ndo_nflag &&
            (addr & f_netmask) == f_localnet) {
@@ -1244,7 +1245,7 @@ init_ipxsaparray(netdissect_options *ndo)
  * Initialize the address to name translation machinery.  We map all
  * non-local IP addresses to numeric addresses if ndo->ndo_fflag is true
  * (i.e., to prevent blocking on the nameserver).  localnet is the IP address
- * of the local network.  mask is its subnet mask.
+ * of the local network, mask is its subnet mask, both in network byte order.
  */
 void
 init_addrtoname(netdissect_options *ndo, uint32_t localnet, uint32_t mask)
diff --git a/print.c b/print.c
index 603969ffce44619cd71a9c361c1c0f068680fc1d..1eb6793bf887c55e8a2f2196c410b46220fe10c8 100644 (file)
--- a/print.c
+++ b/print.c
@@ -241,6 +241,7 @@ static const struct printer printers[] = {
        { NULL,                 0 },
 };
 
+// Both localnet and mask are in network byte order.
 void
 init_print(netdissect_options *ndo, uint32_t localnet, uint32_t mask)
 {
index 918c5f0d355ad2be22073ef84fb356fc6881600f..2c95372529d53f06cfca5866bba5fa422ff92698 100644 (file)
@@ -20,7 +20,7 @@
 .\" WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED WARRANTIES OF
 .\" MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE.
 .\"
-.TH TCPDUMP 1  "30 November 2024"
+.TH TCPDUMP 1  "22 February 2025"
 .SH NAME
 tcpdump \- dump traffic on a network
 .SH SYNOPSIS
@@ -417,12 +417,26 @@ Sun's NIS server \(em usually it hangs forever translating non-local
 internet numbers).
 .IP
 The test for `foreign' IPv4 addresses is done using the IPv4 address and
-netmask of the interface on that capture is being done.  If that
-address or netmask are not available, either because the
-interface on that capture is being done has no address or netmask or
-because it is the "any" pseudo-interface (see the
+netmask of the interface on that capture is being done.  If the interface
+has no IPv4 addresses (which by convention applies to the "any"
+pseudo-interface (see the
 .B \-i
-flag below), this option will not work correctly.
+flag below), the IPv4 netmask is assumed to be /0 and any IPv4 address is
+considered non-foreign.  If the IPv4 netmask is /32, all IPv4 addresses
+except the interface's own address are considered foreign.  If the
+interface has more than one IPv4 address, it is not trivial to predict
+which one will be used for the test.
+.IP
+Without the
+.B \-f
+flag, or when the netmask is assumed to be /0 (as discussed above), the
+.B "ip broadcast"
+primitive in the filter expression matches IPv4 packets that have either
+0.0.0.0 or 255.255.255.255 as the destination address.  With the flag,
+the primitive uses the same netmask (but not the network address) to test
+the IPv4 destination address as the foreign address test.  One exception
+is the netmask /32, in which case the primitive is considered invalid for
+the interface.
 .TP
 .BI \-F " file"
 Use \fIfile\fP as input for the filter expression.
index d420fd254bda2e6344229b3d7f55aeb1109e1398..96e8906b933a6cf00ec38fee3c5d69ee5c18525e 100644 (file)
--- a/tcpdump.c
+++ b/tcpdump.c
@@ -2375,7 +2375,11 @@ DIAG_ON_WARN_UNUSED_RESULT
 #ifdef HAVE_PCAP_SET_OPTIMIZER_DEBUG
        pcap_set_optimizer_debug(dflag);
 #endif
-       if (pcap_compile(pd, &fcode, cmdbuf, Oflag, netmask) < 0)
+       /*
+        * netmask is in network byte order, pcap_compile() takes it
+        * in host byte order.
+        */
+       if (pcap_compile(pd, &fcode, cmdbuf, Oflag, ntohl(netmask)) < 0)
                error("%s", pcap_geterr(pd));
        if (dflag) {
                bpf_dump(&fcode, dflag);
@@ -2390,6 +2394,7 @@ DIAG_ON_WARN_UNUSED_RESULT
                capdns = capdns_setup();
 #endif /* HAVE_CASPER */
 
+       // Both localnet and netmask are in network byte order.
        init_print(ndo, localnet, netmask);
 
 #ifndef _WIN32
@@ -2793,7 +2798,11 @@ DIAG_ON_ASSIGN_ENUM
                                        ndo->ndo_if_printer = get_if_printer(dlt);
                                        /* Free the old filter */
                                        pcap_freecode(&fcode);
-                                       if (pcap_compile(pd, &fcode, cmdbuf, Oflag, netmask) < 0)
+                                       /*
+                                        * netmask is in network byte order, pcap_compile() takes it
+                                        * in host byte order.
+                                        */
+                                       if (pcap_compile(pd, &fcode, cmdbuf, Oflag, ntohl(netmask)) < 0)
                                                error("%s", pcap_geterr(pd));
                                }