Public bug reported:

***** Observed behaviour *****
We ping a host, say 137.226.39.170. While doing so, the router redirects us to 
137.226.42.42. (We verify, with Wireshark and taking a close look at RFC 792, 
that our redirect packet is formatted correctly on-wire.) We see the following 
output of the ping utility:

64 bytes from 137.226.39.170: icmp_seq=46 ttl=251 time=0.619 ms
64 bytes from 137.226.39.170: icmp_seq=47 ttl=251 time=0.676 ms
64 bytes from 137.226.39.170: icmp_seq=48 ttl=251 time=1.92 ms
>From 134.130.3.3 icmp_seq=47 Redirect Host(New nexthop: 42.42.226.137)
64 bytes from 137.226.39.170: icmp_seq=49 ttl=251 time=4.79 ms

The gateway address, as RFC 792 calls it, is displayed the wrong way
around. Feels like an Endianness bug right from the start.


***** Expected behaviour *****
ping utility should display the gateway address correctly. ("New nexthop: 
137.226.42.42")


***** Inspection of source code; bug theory of operation *****
iputils_20190709.orig.tar.xz --> iputils-20190709/ping.c

Bug happens in the function pr_icmph(). This gets called from one of
three different places (lines 899, 1075, and 1101 of ping.c), and only
in one case, the fourth parameter is set to NULL (when called from line
899).

The third parameter ("info") of pr_icmph() is generally meant to be
understood in host byte order. This can be seen in several places (lines
1185, 1284).

Well, but in one place, this fact is ignored and the content of this
third parameter, being in host byte order, gets copied into a structure
that, by definition, is supposed to contain that piece of information in
network byte order, which constitutes the bug.

lines 1250 etc:
        {
                struct sockaddr_in sin = {
                        .sin_family = AF_INET,
                        .sin_addr =  {
                                icp ? icp->un.gateway : info
                        }
                };

                printf(_("(New nexthop: %s)\n"), pr_addr(&sin, sizeof sin));
        }

In the first alternative of the ternary operator, icp->un.gateway is in
network byte order, and the value s_addr within struct in_addr that it
gets copied to is also in network byte order, so everything's beautiful.
But in the second alternative of the ternary operator, i.e. if icp
(parameter number four of pr_icmph) happens to be NULL, as happens when
pr_icmph() gets called from line 899, unfortunately the value of "info"
(in host byte order) gets copied into s_addr, and bad things happen.


***** Bugfix patch *****

--- ping.c              2019-07-09 22:55:49.000000000 +0200
+++ ping_fixed.c        2021-02-09 20:08:18.724747594 +0100
@@ -1251,7 +1251,7 @@
                        struct sockaddr_in sin = {
                                .sin_family = AF_INET,
                                .sin_addr =  {
-                                       icp ? icp->un.gateway : info
+                                       icp ? icp->un.gateway : htonl(info)
                                }
                        };

Simply converts the contents of info (which is in h_ost byte order) to
n_etwork byte order, and everything's fine.

Tried it, and this reliably fixes the bug.

ProblemType: Bug
DistroRelease: Ubuntu 20.04
Package: iputils-ping 3:20190709-3
ProcVersionSignature: Ubuntu 5.4.0-65.73-generic 5.4.78
Uname: Linux 5.4.0-65-generic x86_64
ApportVersion: 2.20.11-0ubuntu27.16
Architecture: amd64
CasperMD5CheckResult: skip
Date: Tue Feb  9 19:33:57 2021
InstallationDate: Installed on 2015-12-09 (1889 days ago)
InstallationMedia: Ubuntu-Server 14.04.3 LTS "Trusty Tahr" - Beta amd64 
(20150805)
SourcePackage: iputils
UpgradeStatus: Upgraded to focal on 2020-12-28 (43 days ago)

** Affects: iputils (Ubuntu)
     Importance: Undecided
         Status: New


** Tags: amd64 apport-bug focal

-- 
You received this bug notification because you are a member of Desktop
Packages, which is subscribed to iputils in Ubuntu.
https://bugs.launchpad.net/bugs/1915191

Title:
  ping utility displays wrong IPv4 address upon receiving ICMP Redirect

Status in iputils package in Ubuntu:
  New

Bug description:
  ***** Observed behaviour *****
  We ping a host, say 137.226.39.170. While doing so, the router redirects us 
to 137.226.42.42. (We verify, with Wireshark and taking a close look at RFC 
792, that our redirect packet is formatted correctly on-wire.) We see the 
following output of the ping utility:

  64 bytes from 137.226.39.170: icmp_seq=46 ttl=251 time=0.619 ms
  64 bytes from 137.226.39.170: icmp_seq=47 ttl=251 time=0.676 ms
  64 bytes from 137.226.39.170: icmp_seq=48 ttl=251 time=1.92 ms
  From 134.130.3.3 icmp_seq=47 Redirect Host(New nexthop: 42.42.226.137)
  64 bytes from 137.226.39.170: icmp_seq=49 ttl=251 time=4.79 ms

  The gateway address, as RFC 792 calls it, is displayed the wrong way
  around. Feels like an Endianness bug right from the start.

  
  ***** Expected behaviour *****
  ping utility should display the gateway address correctly. ("New nexthop: 
137.226.42.42")

  
  ***** Inspection of source code; bug theory of operation *****
  iputils_20190709.orig.tar.xz --> iputils-20190709/ping.c

  Bug happens in the function pr_icmph(). This gets called from one of
  three different places (lines 899, 1075, and 1101 of ping.c), and only
  in one case, the fourth parameter is set to NULL (when called from
  line 899).

  The third parameter ("info") of pr_icmph() is generally meant to be
  understood in host byte order. This can be seen in several places
  (lines 1185, 1284).

  Well, but in one place, this fact is ignored and the content of this
  third parameter, being in host byte order, gets copied into a
  structure that, by definition, is supposed to contain that piece of
  information in network byte order, which constitutes the bug.

  lines 1250 etc:
          {
                  struct sockaddr_in sin = {
                          .sin_family = AF_INET,
                          .sin_addr =  {
                                  icp ? icp->un.gateway : info
                          }
                  };

                  printf(_("(New nexthop: %s)\n"), pr_addr(&sin, sizeof sin));
          }

  In the first alternative of the ternary operator, icp->un.gateway is
  in network byte order, and the value s_addr within struct in_addr that
  it gets copied to is also in network byte order, so everything's
  beautiful. But in the second alternative of the ternary operator, i.e.
  if icp (parameter number four of pr_icmph) happens to be NULL, as
  happens when pr_icmph() gets called from line 899, unfortunately the
  value of "info" (in host byte order) gets copied into s_addr, and bad
  things happen.

  
  ***** Bugfix patch *****

  --- ping.c            2019-07-09 22:55:49.000000000 +0200
  +++ ping_fixed.c      2021-02-09 20:08:18.724747594 +0100
  @@ -1251,7 +1251,7 @@
                        struct sockaddr_in sin = {
                                .sin_family = AF_INET,
                                .sin_addr =  {
  -                                     icp ? icp->un.gateway : info
  +                                     icp ? icp->un.gateway : htonl(info)
                                }
                        };

  Simply converts the contents of info (which is in h_ost byte order) to
  n_etwork byte order, and everything's fine.

  Tried it, and this reliably fixes the bug.

  ProblemType: Bug
  DistroRelease: Ubuntu 20.04
  Package: iputils-ping 3:20190709-3
  ProcVersionSignature: Ubuntu 5.4.0-65.73-generic 5.4.78
  Uname: Linux 5.4.0-65-generic x86_64
  ApportVersion: 2.20.11-0ubuntu27.16
  Architecture: amd64
  CasperMD5CheckResult: skip
  Date: Tue Feb  9 19:33:57 2021
  InstallationDate: Installed on 2015-12-09 (1889 days ago)
  InstallationMedia: Ubuntu-Server 14.04.3 LTS "Trusty Tahr" - Beta amd64 
(20150805)
  SourcePackage: iputils
  UpgradeStatus: Upgraded to focal on 2020-12-28 (43 days ago)

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/iputils/+bug/1915191/+subscriptions

-- 
Mailing list: https://launchpad.net/~desktop-packages
Post to     : desktop-packages@lists.launchpad.net
Unsubscribe : https://launchpad.net/~desktop-packages
More help   : https://help.launchpad.net/ListHelp

Reply via email to