RIP: Fix two loops for undefined behavior at runtime

1) RIPv2
The error was:
print-rip.c:386:9: runtime error: unsigned integer overflow: 16 - 20
cannot be represented in type 'unsigned int'

Without this change the unsigned integer variable 'len' is assigned a
very high value, because of underflow, and the loop continue incorrectly.

Add a test case.

2) RIPv1
Same bugfix, based on a code inspection, so comes without a test case.

diff --git a/print-rip.c b/print-rip.c
index a26961b744e978cf7084c980439422df3487a8d8..286497a693d89252074f0b563fd2438ac4b0f775 100644 (file)
--- a/print-rip.c
+++ b/print-rip.c
@@ -360,6 +360,12 @@ rip_print(netdissect_options *ndo,
                                        nd_print_trunc(ndo);
                                        break;
                                }
+                               if (len < entry_size) {
+                                       ND_PRINT(" [len(%u) < entry_size(%u)]",
+                                                len, entry_size);
+                                       nd_print_invalid(ndo);
+                                       break;
+                               }
                                p += entry_size;
                                len -= entry_size;
                        }
@@ -376,12 +382,12 @@ rip_print(netdissect_options *ndo,
                                        nd_print_trunc(ndo);
                                        break;
                                }
-#if 0
                                if (len < entry_size) {
-                                       ND_PRINT("WTF?");
+                                       ND_PRINT(" [len(%u) < entry_size(%u)]",
+                                                len, entry_size);
+                                       nd_print_invalid(ndo);
                                        break;
                                }
-#endif
                                p += entry_size;
                                len -= entry_size;
                        }

diff --git a/tests/TESTLIST b/tests/TESTLIST
index d39928aaa951b796836c7fa648f421c7bb33fc6a..ef7e282024cf89372c76b18e98f41655e5a9cd44 100644 (file)
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -163,6 +163,9 @@ rpvst-v             rpvstp-trunk-native-vid5.pcap   rpvst-v.out     -v
 ripv1v2         ripv1v2.pcap            ripv1v2.out     -v
 ripv2_auth      ripv2_auth.pcap         ripv2_auth.out  -v
 
+# RIP invalid
+ripv2-invalid-length ripv2-invalid-length.pcap ripv2-invalid-length.out -v
+
 # DHCPv6 tests
 dhcpv6-aftr-name       dhcpv6-AFTR-Name-RFC6334.pcap   dhcpv6-AFTR-Name-RFC6334.out    -v
 dhcpv6-ia-na   dhcpv6-ia-na.pcap       dhcpv6-ia-na.out        -v

diff --git a/tests/ripv2-invalid-length.out b/tests/ripv2-invalid-length.out
new file mode 100644 (file)
index 0000000..f8155dd
--- /dev/null
+++ b/tests/ripv2-invalid-length.out
@@ -0,0 +1,12 @@
+    1  08:36:15.227124 IP (tos 0xc0, ttl 2, id 0, offset 0, flags [none], proto UDP (17), length 192)
+    10.7.56.254.520 > 224.0.0.9.520: 
+       RIPv2, Response, length: 160, routes: 8 or less
+         AFI IPv4,        10.7.0.0/24, tag 0x0000, metric: 1, next-hop: self
+         AFI IPv4,       10.7.41.0/24, tag 0x0000, metric: 1, next-hop: self
+         AFI IPv4,       10.7.51.0/24, tag 0x0000, metric: 1, next-hop: self
+         AFI IPv4,       10.7.52.0/25, tag 0x0000, metric: 1, next-hop: self
+         AFI IPv4,       10.7.53.0/24, tag 0x0000, metric: 1, next-hop: self
+         AFI IPv4,       10.7.57.0/24, tag 0x0000, metric: 268435457, next-hop: self
+         AFI IPv4,       10.7.61.0/24, tag 0x0000, metric: 1, next-hop: self
+         AFI Unknown (37)
+         0x0000:  5100 0000 ff00 0000 0000 0000 0000 0002 [len(16) < entry_size(20)] (invalid)

diff --git a/tests/ripv2-invalid-length.pcap b/tests/ripv2-invalid-length.pcap
new file mode 100644 (file)
index 0000000..db475f2
Binary files /dev/null and b/tests/ripv2-invalid-length.pcap differ