AddressSanitizer: 2 heap-buffer-overflow problems (packet2tree() && get_l2len())

[Marsman1996]

Both tested in Ubuntu 18.04, 64bit, gcc 7.3.0, tcpreplay (master 2d87447). And tcpprep -V returns

tcpprep version: 4.3.0 (build git:v4.3.0-1-g2d874470)
Copyright 2013-2018 by Fred Klassen <tcpreplay at appneta dot com> - AppNeta
Copyright 2000-2012 by Aaron Turner <aturner at synfin dot net>
The entire Tcpreplay Suite is licensed under the GPLv3
Cache file supported: 04
Not compiled with libdnet.
Compiled against libpcap: 1.8.1
64 bit packet counters: enabled
Verbose printing via tcpdump: enabled

Triggered by
./tcpprep --auto=bridge --pcap=$POC --cachefile=/dev/null

POC1

poc file:
https://github.com/Marsman1996/pocs/blob/master/tcpreplay/poc15-packet2tree-heapoverflow

ASAN info:

ubuntu@ubuntu-virtual-machine:~/Desktop/crashana$ ./tcpreplay/tcpreplay-2d87447/bin_asan/bin/tcpprep --auto=bridge --pcap=./tcpreplay/poc15-packet2tree-heapoverflow --cachefile=/dev/null
Warning: ./tcpreplay/poc15-packet2tree-heapoverflow was captured using a snaplen of 50 bytes.  This may mean you have truncated packets.
=================================================================
==46592==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60600000005a at pc 0x5625e6ae8e31 bp 0x7ffc780aa970 sp 0x7ffc780aa960
READ of size 4 at 0x60600000005a thread T0
    #0 0x5625e6ae8e30 in packet2tree ../../src/tree.c:749
    #1 0x5625e6aebfe2 in add_tree_ipv4 ../../src/tree.c:536
    #2 0x5625e6ae6fef in process_raw_packets ../../src/tcpprep.c:463
    #3 0x5625e6ae4bed in main ../../src/tcpprep.c:146
    #4 0x7fc706f92b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
    #5 0x5625e6ae54e9 in _start (/home/ubuntu/Desktop/crashana/tcpreplay/tcpreplay-2d87447/bin_asan/bin/tcpprep+0x104e9)

0x60600000005a is located 8 bytes to the right of 50-byte region [0x606000000020,0x606000000052)
allocated by thread T0 here:
    #0 0x7fc707681b50 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb50)
    #1 0x7fc70738189f  (/usr/lib/x86_64-linux-gnu/libpcap.so.0.8+0x1f89f)

SUMMARY: AddressSanitizer: heap-buffer-overflow ../../src/tree.c:749 in packet2tree
Shadow bytes around the buggy address:
  0x0c0c7fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c0c7fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c0c7fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c0c7fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c0c7fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c0c7fff8000: fa fa fa fa 00 00 00 00 00 00 02[fa]fa fa fa fa
  0x0c0c7fff8010: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c7fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c7fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c7fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c7fff8050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==46592==ABORTING

POC2

poc file:
https://github.com/Marsman1996/pocs/blob/master/tcpreplay/poc16-get_l2len-heapoverflow

ASAN info:

ubuntu@ubuntu-virtual-machine:~/Desktop/crashana$ ./tcpreplay/tcpreplay-2d87447/bin_asan/bin/tcpprep --auto=bridge --pcap=./tcpreplay/poc16-get_l2len-heapoverflow --cachefile=/dev/null
Warning: ./tcpreplay/poc16-get_l2len-heapoverflow was captured using a snaplen of 17 bytes.  This may mean you have truncated packets.
=================================================================
==54318==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000000020 at pc 0x5637ffc1f43a bp 0x7fffbeb468b0 sp 0x7fffbeb468a0
READ of size 2 at 0x603000000020 thread T0
    #0 0x5637ffc1f439 in get_l2len ../../../src/common/get.c:183
    #1 0x5637ffc1f492 in get_ipv4 ../../../src/common/get.c:247
    #2 0x5637ffc12c79 in process_raw_packets ../../src/tcpprep.c:367
    #3 0x5637ffc10bed in main ../../src/tcpprep.c:146
    #4 0x7f99179dfb96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
    #5 0x5637ffc114e9 in _start (/home/ubuntu/Desktop/crashana/tcpreplay/tcpreplay-2d87447/bin_asan/bin/tcpprep+0x104e9)

0x603000000021 is located 0 bytes to the right of 17-byte region [0x603000000010,0x603000000021)
allocated by thread T0 here:
    #0 0x7f99180ceb50 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb50)
    #1 0x7f9917dce89f  (/usr/lib/x86_64-linux-gnu/libpcap.so.0.8+0x1f89f)

SUMMARY: AddressSanitizer: heap-buffer-overflow ../../../src/common/get.c:183 in get_l2len
Shadow bytes around the buggy address:
  0x0c067fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c067fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c067fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c067fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c067fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c067fff8000: fa fa 00 00[01]fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff8010: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff8050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==54318==ABORTING

[fklassen]

fixed in PR #532

[carnil]

Two CVEs were assigned here: CVE-2018-20552 and CVE-2018-20553.

[lvtao-sec]

Hi，your patch of CVE-2018-20552 isn't right. The crash occurs at the following code:

    873       * ICMP
    874       */
    875      else if (proto == IPPROTO_ICMP) {
    876
    877          /* prevent alignment issues */
                 // data=0x00007ffffffed8d8  →  [...]  →  0x0000800000010000, hl=0x2c
 →  878          memcpy(&icmp_hdr, (data + TCPR_ETH_H + hl), TCPR_ICMPV4_H);
    879
    880          dbgx(3, "%s uses ICMP...  ", srcip);
    881
    882          /*
    883           * if port unreachable, then source == server, dst == client
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── threads ────
[#0] Id 1, Name: "tcpprep", stopped, reason: BREAKPOINT
───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── trace ────
[#0] 0x51c4e8 → packet2tree(data=0x606000000020 "", len=0x32)
[#1] 0x51b3cc → add_tree_ipv4(ip=0xcbcbcbcb, data=0x606000000020 "", len=0x32)
[#2] 0x5111e9 → process_raw_packets(pcap=0x616000000380)
[#3] 0x50fa14 → main(argc=0x4, argv=0x7fffffffe458)
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
gef➤  p len
$9 = 0x32
gef➤  p hl
$10 = 0x2c
gef➤  p hl+0xe
$11 = 0x3a
gef➤

The overflow is caused by the line tree.c:751, which doesn't check the ip_hl whether proper.

        memcpy(&ip_hdr, (data + TCPR_ETH_H + hl), TCPR_IPV4_H);

        node->family = AF_INET;
        node->u.ip = ip_hdr.ip_src.s_addr;
        proto = ip_hdr.ip_p;
        hl += ip_hdr.ip_hl * 4;

As this poc shows, it made the ip_hl to be 0xb, which is bigger than the right one.

gef➤  p ip_hdr
$8 = {
  ip_hl = 0xb,
  ip_v = 0xc,
  ip_tos = 0xcb,
  ip_len = 0xcbcb,
  ip_id = 0x32,
  ip_off = 0x0,
  ip_ttl = 0xcb,
  ip_p = 0x1,
  ip_sum = 0x0,
  ip_src = {
    s_addr = 0xcbcbcbcb
  },
  ip_dst = {
    s_addr = 0xcbcbcbcb
  }
}

[fklassen]

Reopening to investigate

[lvtao-sec]

The patch for CVE-2018-20553 is also wrong.
This issue is an error of reading out of the heap buffer. When the program read structure vlan_hdr_t, it doesn't check the relationship of length between vlan_hdr_t and datalen . The above poc can make 'vlan_hdr_t' larger than datalen, which causes the read out of bound.

─
    178          if ((size_t)datalen >= sizeof(eth_hdr_t) + l2_len) {
    179              uint16_t ether_type = ntohs(((eth_hdr_t*)(pktdata + l2_len))->ether_type);
    180
    181              while (ether_type == ETHERTYPE_VLAN) {
    182                  vlan_hdr_t *vlan_hdr = (vlan_hdr_t *)(pktdata + l2_len);
                         // vlan_hdr=0x00007ffffffed9b8  →  [...]  →  0x0000000000000000
 →  183                  ether_type = ntohs(vlan_hdr->vlan_len);
    184                  l2_len += 4;
    185                  if ((size_t)datalen < sizeof(vlan_hdr_t) + l2_len) {
    186                      l2_len = -1;
    187                      break;
    188                  }
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── threads ────
[#0] Id 1, Name: "tcpprep", stopped, reason: BREAKPOINT
───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── trace ────
[#0] 0x528fa1 → get_l2len(pktdata=0x603000000010 "", datalen=0x11, datalink=0x1)
[#1] 0x529232 → get_ipv4(pktdata=0x603000000010 "", datalen=0x11, datalink=0x1, newbuff=0x7fffffffdf50)
[#2] 0x510461 → process_raw_packets(pcap=0x616000000380)
[#3] 0x50fa14 → main(argc=0x4, argv=0x7fffffffe508)
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
gef➤  p vlan_hdr
$16 = (vlan_hdr_t *) 0x603000000010
gef➤  p &(vlan_hdr->vlan_len)   //notice: vlan_len is 2 bytes
$17 = (uint16_t *) 0x603000000020
gef➤

[fklassen]

Fixed first issue in PR #609 and second issue was fixed in #584