CVE-2017-5342/pass correct caplen value to ether_print()

In that function the "length" parameter means off-the-wire length, that
is, the length declared inside the outer header. The "caplen" parameter
means the amount of bytes actually available in the captured packet.

gre_print_0() and the functions modelled after it passed the value of
"length" instead of the value of "caplen", this could make ether_print()
access beyond the memory allocated for the captured packet. Brian
Carpenter had demonstrated this for the OTV case.

Fix the involved functions that call ether_print() to pass the correct
value and leave a comment to dismiss "caplen" later as its value can be
reliably derived from the other ether_print() parameters.

diff --git a/print-ether.c b/print-ether.c
index 5072c442f74240bf64c89e1aa97e5351e9bdb9a8..241c5070c9596896f9c02d835e5ad383089fb293 100644 (file)
--- a/print-ether.c
+++ b/print-ether.c
@@ -126,6 +126,8 @@ ether_hdr_print(netdissect_options *ndo,
  * This might be encapsulated within another frame; we might be passed
  * a pointer to a function that can print header information for that
  * frame's protocol, and an argument to pass to that function.
  * This might be encapsulated within another frame; we might be passed
  * a pointer to a function that can print header information for that
  * frame's protocol, and an argument to pass to that function.

+ *
+ * FIXME: caplen can and should be derived from ndo->ndo_snapend and p.

  */
 u_int
 ether_print(netdissect_options *ndo,
  */
 u_int
 ether_print(netdissect_options *ndo,

diff --git a/print-geneve.c b/print-geneve.c
index f0319636690787bbbd9b6be562b2cda70678ec21..d1ed242fda6ab533c9af6ace3eb8f2c8cdee08d7 100644 (file)
--- a/print-geneve.c
+++ b/print-geneve.c
@@ -225,7 +225,7 @@ geneve_print(netdissect_options *ndo, const u_char *bp, u_int len)
 
     if (ethertype_print(ndo, prot, bp, len, len, NULL, NULL) == 0) {
         if (prot == ETHERTYPE_TEB)
 
     if (ethertype_print(ndo, prot, bp, len, len, NULL, NULL) == 0) {
         if (prot == ETHERTYPE_TEB)

-            ether_print(ndo, bp, len, len, NULL, NULL);
+            ether_print(ndo, bp, len, ndo->ndo_snapend - bp, NULL, NULL);

         else
             ND_PRINT((ndo, "geneve-proto-0x%x", prot));
     }
         else
             ND_PRINT((ndo, "geneve-proto-0x%x", prot));
     }

diff --git a/print-gre.c b/print-gre.c
index 4b83ca2599dc1efc5dcb7aff9d2ffce7c365efba..e0f60c944dd0ef13f83f0dea76d8a20c5b41f212 100644 (file)
--- a/print-gre.c
+++ b/print-gre.c
@@ -229,7 +229,7 @@ gre_print_0(netdissect_options *ndo, const u_char *bp, u_int length)
                isoclns_print(ndo, bp, len, len);
                break;
        case ETHERTYPE_TEB:
                isoclns_print(ndo, bp, len, len);
                break;
        case ETHERTYPE_TEB:

-               ether_print(ndo, bp, len, len, NULL, NULL);
+               ether_print(ndo, bp, len, ndo->ndo_snapend - bp, NULL, NULL);

                break;
        default:
                ND_PRINT((ndo, "gre-proto-0x%x", prot));
                break;
        default:
                ND_PRINT((ndo, "gre-proto-0x%x", prot));

diff --git a/print-nsh.c b/print-nsh.c
index 82976a03a029fa4e3fe84c1009454ec1676b9b03..abd722d40454db9b30454e2a567ae0805c67de54 100644 (file)
--- a/print-nsh.c
+++ b/print-nsh.c
@@ -170,7 +170,7 @@ nsh_print(netdissect_options *ndo, const u_char *bp, u_int len)
         ip6_print(ndo, bp, next_len);
         break;
     case 0x3:
         ip6_print(ndo, bp, next_len);
         break;
     case 0x3:

-        ether_print(ndo, bp, next_len, next_len, NULL, NULL);
+        ether_print(ndo, bp, next_len, ndo->ndo_snapend - bp, NULL, NULL);

         break;
     default:
         ND_PRINT((ndo, "ERROR: unknown-next-protocol"));
         break;
     default:
         ND_PRINT((ndo, "ERROR: unknown-next-protocol"));

diff --git a/print-otv.c b/print-otv.c
index f276441ffb2d611561985a3d795e87cb2c0e19bb..5a82752c19a2b933b5818b7a6a7d43d36bda0313 100644 (file)
--- a/print-otv.c
+++ b/print-otv.c
@@ -64,7 +64,7 @@ otv_print(netdissect_options *ndo, const u_char *bp, u_int len)
     ND_TCHECK(*bp);
     bp += 1;
 
     ND_TCHECK(*bp);
     bp += 1;
 

-    ether_print(ndo, bp, len - 8, len - 8, NULL, NULL);
+    ether_print(ndo, bp, len - 8, ndo->ndo_snapend - bp, NULL, NULL);

     return;
 
 trunc:
     return;
 
 trunc:

diff --git a/print-vxlan-gpe.c b/print-vxlan-gpe.c
index a3c214d3ef5b34e0bb225cff82ede5407fe20055..6d170de10e6a2bd9fd2aef2ebf82ab0b999b41e1 100644 (file)
--- a/print-vxlan-gpe.c
+++ b/print-vxlan-gpe.c
@@ -92,7 +92,7 @@ vxlan_gpe_print(netdissect_options *ndo, const u_char *bp, u_int len)
         ip6_print(ndo, bp, len - 8);
         break;
     case 0x3:
         ip6_print(ndo, bp, len - 8);
         break;
     case 0x3:

-        ether_print(ndo, bp, len - 8, len - 8, NULL, NULL);
+        ether_print(ndo, bp, len - 8, ndo->ndo_snapend - bp, NULL, NULL);

         break;
     case 0x4:
         nsh_print(ndo, bp, len - 8);
         break;
     case 0x4:
         nsh_print(ndo, bp, len - 8);

diff --git a/print-vxlan.c b/print-vxlan.c
index 052cbe3d45f7cd6b527e8dd9328b266a7dd9881a..0c2a82e2afeda5498b4a876d27f96aa8a5ae4131 100644 (file)
--- a/print-vxlan.c
+++ b/print-vxlan.c
@@ -65,7 +65,7 @@ vxlan_print(netdissect_options *ndo, const u_char *bp, u_int len)
     ND_PRINT((ndo, "flags [%s] (0x%02x), ", flags & 0x08 ? "I" : ".", flags));
     ND_PRINT((ndo, "vni %u\n", vni));
 
     ND_PRINT((ndo, "flags [%s] (0x%02x), ", flags & 0x08 ? "I" : ".", flags));
     ND_PRINT((ndo, "vni %u\n", vni));
 

-    ether_print(ndo, bp, len - VXLAN_HDR_LEN, len - VXLAN_HDR_LEN, NULL, NULL);
+    ether_print(ndo, bp, len - VXLAN_HDR_LEN, ndo->ndo_snapend - bp, NULL, NULL);

 
     return;
 
 
     return;
 

diff --git a/tests/TESTLIST b/tests/TESTLIST
index 379dae5d8bcba8e67c9154397664bdba2e6e80fa..5046046c7b30e70874dc60610ba3f0b5692ed90a 100644 (file)
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -423,6 +423,7 @@ relts-0x80000000    relts-0x80000000.pcap   relts-0x80000000.out    -t -v -n
 ipv6hdr-heapoverflow   ipv6hdr-heapoverflow.pcap       ipv6hdr-heapoverflow.out        -t
 ipv6hdr-heapoverflow-v ipv6hdr-heapoverflow.pcap       ipv6hdr-heapoverflow-v.out      -t -v
 otv-heapoverflow-1     otv-heapoverflow-1.pcap         otv-heapoverflow-1.out          -t -c10
 ipv6hdr-heapoverflow   ipv6hdr-heapoverflow.pcap       ipv6hdr-heapoverflow.out        -t
 ipv6hdr-heapoverflow-v ipv6hdr-heapoverflow.pcap       ipv6hdr-heapoverflow-v.out      -t -v
 otv-heapoverflow-1     otv-heapoverflow-1.pcap         otv-heapoverflow-1.out          -t -c10

+otv-heapoverflow-2     otv-heapoverflow-2.pcap         otv-heapoverflow-2.out          -t -c10

 
 # RTP tests
 # fuzzed pcap
 
 # RTP tests
 # fuzzed pcap

diff --git a/tests/otv-heapoverflow-2.out b/tests/otv-heapoverflow-2.out
new file mode 100644 (file)
index 0000000..7ea809f
--- /dev/null
+++ b/tests/otv-heapoverflow-2.out
@@ -0,0 +1,11 @@
+IP 192.168.0.134.47808 > 192.168.0.24.47808: UDP, length 6
+IP 192.168.0.134.47808 > 192.168.0.24.47808: UDP, length 12
+IP 192.168.0.24.47808 > 192.168.0.134.47808: UDP, length 6
+IP 192.168.0.24.47808 > 192.168.0.255.47808: UDP, length 18
+IP 192.168.0.105.47808 > 192.168.0.255.47808: UDP, length 25
+IP 192.168.0.24.47808 > 192.168.0.134.47808: UDP, length 31
+IP 192.168.0.18.47808 > 192.168.0.255.47808: UDP, length 24
+IP 192.168.0.24.40896 > 192.168.0.134.47808: UDP, length 30
+IP 192.168.0.24.47808 > 192.168.0.255.47808: UDP, length 20
+IP 192.168.0.9.37123 > 97.34.1.224.8472: OTV, flags [I] (0x9d), overlay 12124160, instance 4587520
+[|ether]

diff --git a/tests/otv-heapoverflow-2.pcap b/tests/otv-heapoverflow-2.pcap
new file mode 100644 (file)
index 0000000..69d6e78
Binary files /dev/null and b/tests/otv-heapoverflow-2.pcap differ