|
| 1 | +From 33ebd21ac99d3e6ab2d51b6581cbec7e9fba17b6 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Petr Vorel < [email protected]> |
| 3 | +Date: Mon, 5 May 2025 23:55:57 +0200 |
| 4 | +Subject: [PATCH] ping: Fix signed 64-bit integer overflow in RTT calculation |
| 5 | + |
| 6 | +Crafted ICMP Echo Reply packet can cause signed integer overflow in |
| 7 | + |
| 8 | +1) triptime calculation: |
| 9 | +triptime = tv->tv_sec * 1000000 + tv->tv_usec; |
| 10 | + |
| 11 | +2) tsum2 increment which uses triptime |
| 12 | +rts->tsum2 += (double)((long long)triptime * (long long)triptime); |
| 13 | + |
| 14 | +3) final tmvar: |
| 15 | +tmvar = (rts->tsum2 / total) - (tmavg * tmavg) |
| 16 | + |
| 17 | + $ export CFLAGS="-O1 -g -fsanitize=address,undefined -fno-omit-frame-pointer" |
| 18 | + $ export LDFLAGS="-fsanitize=address,undefined -fno-omit-frame-pointer" |
| 19 | + $ meson setup .. -Db_sanitize=address,undefined |
| 20 | + $ ninja |
| 21 | + $ ./ping/ping -c2 127.0.0.1 |
| 22 | + |
| 23 | + PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data. |
| 24 | + 64 bytes from 127.0.0.1: icmp_seq=1 ttl=64 time=0.061 ms |
| 25 | + ../ping/ping_common.c:757:25: runtime error: signed integer overflow: -2513732689199106 * 1000000 cannot be represented in type 'long int' |
| 26 | + ../ping/ping_common.c:757:12: runtime error: signed integer overflow: -4975495174606980224 + -6510615555425289427 cannot be represented in type 'long int' |
| 27 | + ../ping/ping_common.c:769:47: runtime error: signed integer overflow: 6960633343677281965 * 6960633343677281965 cannot be represented in type 'long int' |
| 28 | + 24 bytes from 127.0.0.1: icmp_seq=1 ttl=64 (truncated) |
| 29 | + ./ping/ping: Warning: time of day goes back (-7256972569576721377us), taking countermeasures |
| 30 | + ./ping/ping: Warning: time of day goes back (-7256972569576721232us), taking countermeasures |
| 31 | + 24 bytes from 127.0.0.1: icmp_seq=1 ttl=64 (truncated) |
| 32 | + ../ping/ping_common.c:265:16: runtime error: signed integer overflow: 6960633343677281965 * 2 cannot be represented in type 'long int' |
| 33 | + 64 bytes from 127.0.0.1: icmp_seq=2 ttl=64 time=0.565 ms |
| 34 | + |
| 35 | + --- 127.0.0.1 ping statistics --- |
| 36 | + 2 packets transmitted, 2 received, +2 duplicates, 0% packet loss, time 1002ms |
| 37 | + ../ping/ping_common.c:940:42: runtime error: signed integer overflow: 1740158335919320832 * 1740158335919320832 cannot be represented in type 'long int' |
| 38 | + rtt min/avg/max/mdev = 0.000/1740158335919320.832/6960633343677281.965/-1623514645242292.-224 ms |
| 39 | + |
| 40 | +To fix the overflow check allowed ranges of struct timeval members: |
| 41 | +* tv_sec <0, LONG_MAX/1000000> |
| 42 | +* tv_usec <0, 999999> |
| 43 | + |
| 44 | +Fix includes 2 new error messages (needs translation). |
| 45 | +Also existing message "time of day goes back ..." needed to be modified |
| 46 | +as it now prints tv->tv_sec which is a second (needs translation update). |
| 47 | + |
| 48 | +After fix: |
| 49 | + |
| 50 | + $ ./ping/ping -c2 127.0.0.1 |
| 51 | + 64 bytes from 127.0.0.1: icmp_seq=1 ttl=64 time=0.057 ms |
| 52 | + ./ping/ping: Warning: invalid tv_usec -6510615555424928611 us |
| 53 | + ./ping/ping: Warning: time of day goes back (-3985394643238914 s), taking countermeasures |
| 54 | + ./ping/ping: Warning: invalid tv_usec -6510615555424928461 us |
| 55 | + ./ping/ping: Warning: time of day goes back (-3985394643238914 s), taking countermeasures |
| 56 | + 24 bytes from 127.0.0.1: icmp_seq=1 ttl=64 (truncated) |
| 57 | + ./ping/ping: Warning: invalid tv_usec -6510615555425884541 us |
| 58 | + ./ping/ping: Warning: time of day goes back (-4243165695442945 s), taking countermeasures |
| 59 | + 24 bytes from 127.0.0.1: icmp_seq=1 ttl=64 (truncated) |
| 60 | + 64 bytes from 127.0.0.1: icmp_seq=2 ttl=64 time=0.111 ms |
| 61 | + |
| 62 | + --- 127.0.0.1 ping statistics --- |
| 63 | + 2 packets transmitted, 2 received, +2 duplicates, 0% packet loss, time 101ms |
| 64 | + rtt min/avg/max/mdev = 0.000/0.042/0.111/0.046 ms |
| 65 | + |
| 66 | +Fixes: https://github.com/iputils/iputils/issues/584 |
| 67 | +Fixes: CVE-2025-472 |
| 68 | +Link: https://github.com/Zephkek/ping-rtt-overflow/ |
| 69 | +Co-developed-by: Cyril Hrubis < [email protected]> |
| 70 | +Reported-by: Mohamed Maatallah < [email protected]> |
| 71 | +Reviewed-by: Mohamed Maatallah < [email protected]> |
| 72 | +Reviewed-by: Cyril Hrubis < [email protected]> |
| 73 | +Signed-off-by: Petr Vorel < [email protected]> |
| 74 | +Signed-off-by: Azure Linux Security Servicing Account < [email protected]> |
| 75 | +Upstream-reference: https://github.com/iputils/iputils/pull/585/commits/b41e4a10ab1f749a9bd149c608213c9704c3147f.patch |
| 76 | +--- |
| 77 | + iputils_common.h | 3 +++ |
| 78 | + ping/ping_common.c | 22 +++++++++++++++++++--- |
| 79 | + 2 files changed, 22 insertions(+), 3 deletions(-) |
| 80 | + |
| 81 | +diff --git a/iputils_common.h b/iputils_common.h |
| 82 | +index 49e790d..829a749 100644 |
| 83 | +--- a/iputils_common.h |
| 84 | ++++ b/iputils_common.h |
| 85 | +@@ -10,6 +10,9 @@ |
| 86 | + !!__builtin_types_compatible_p(__typeof__(arr), \ |
| 87 | + __typeof__(&arr[0]))])) * 0) |
| 88 | + |
| 89 | ++/* 1000001 = 1000000 tv_sec + 1 tv_usec */ |
| 90 | ++#define TV_SEC_MAX_VAL (LONG_MAX/1000001) |
| 91 | ++ |
| 92 | + #ifdef __GNUC__ |
| 93 | + # define iputils_attribute_format(t, n, m) __attribute__((__format__ (t, n, m))) |
| 94 | + #else |
| 95 | +diff --git a/ping/ping_common.c b/ping/ping_common.c |
| 96 | +index 73da26c..f44b2c0 100644 |
| 97 | +--- a/ping/ping_common.c |
| 98 | ++++ b/ping/ping_common.c |
| 99 | +@@ -744,16 +744,32 @@ int gather_statistics(struct ping_rts *rts, uint8_t *icmph, int icmplen, |
| 100 | + |
| 101 | + restamp: |
| 102 | + tvsub(tv, &tmp_tv); |
| 103 | +- triptime = tv->tv_sec * 1000000 + tv->tv_usec; |
| 104 | +- if (triptime < 0) { |
| 105 | +- error(0, 0, _("Warning: time of day goes back (%ldus), taking countermeasures"), triptime); |
| 106 | ++ |
| 107 | ++ if (tv->tv_usec >= 1000000) { |
| 108 | ++ error(0, 0, _("Warning: invalid tv_usec %ld us"), tv->tv_usec); |
| 109 | ++ tv->tv_usec = 999999; |
| 110 | ++ } |
| 111 | ++ |
| 112 | ++ if (tv->tv_usec < 0) { |
| 113 | ++ error(0, 0, _("Warning: invalid tv_usec %ld us"), tv->tv_usec); |
| 114 | ++ tv->tv_usec = 0; |
| 115 | ++ } |
| 116 | ++ |
| 117 | ++ if (tv->tv_sec > TV_SEC_MAX_VAL) { |
| 118 | ++ error(0, 0, _("Warning: invalid tv_sec %ld s"), tv->tv_sec); |
| 119 | ++ triptime = 0; |
| 120 | ++ } else if (tv->tv_sec < 0) { |
| 121 | ++ error(0, 0, _("Warning: time of day goes back (%ld s), taking countermeasures"), tv->tv_sec); |
| 122 | + triptime = 0; |
| 123 | + if (!rts->opt_latency) { |
| 124 | + gettimeofday(tv, NULL); |
| 125 | + rts->opt_latency = 1; |
| 126 | + goto restamp; |
| 127 | + } |
| 128 | ++ } else { |
| 129 | ++ triptime = tv->tv_sec * 1000000 + tv->tv_usec; |
| 130 | + } |
| 131 | ++ |
| 132 | + if (!csfailed) { |
| 133 | + rts->tsum += triptime; |
| 134 | + rts->tsum2 += (double)((long long)triptime * (long long)triptime); |
| 135 | +-- |
| 136 | +2.45.4 |
| 137 | + |
0 commit comments