Skip to content
Permalink
Browse files

CVE-2017-13688/OLSR: Do bounds checks before we fetch data.

While we're at it, clean up some other bounds checks, so we check that
we have a complete IPv4 message header if it's IPv4 and a complete IPv6
message header if it's IPv6.

This fixes a buffer over-read discovered by Bhargava Shastry,
SecT/TU Berlin.

Add tests using the capture files supplied by the reporter(s).
  • Loading branch information...
guyharris authored and infrastation committed Aug 24, 2017
1 parent 26b9567 commit 0cb1b8a434b599b8d636db029aadb757c24e39d6
@@ -359,10 +359,9 @@ olsr_print(netdissect_options *ndo,
} msgptr;
int msg_len_valid = 0;

ND_TCHECK2(*tptr, sizeof(struct olsr_msg4));

if (is_ipv6)
{
ND_TCHECK2(*tptr, sizeof(struct olsr_msg6));
msgptr.v6 = (const struct olsr_msg6 *) tptr;
msg_type = msgptr.v6->msg_type;
msg_len = EXTRACT_16BITS(msgptr.v6->msg_len);
@@ -393,6 +392,7 @@ olsr_print(netdissect_options *ndo,
}
else /* (!is_ipv6) */
{
ND_TCHECK2(*tptr, sizeof(struct olsr_msg4));
msgptr.v4 = (const struct olsr_msg4 *) tptr;
msg_type = msgptr.v4->msg_type;
msg_len = EXTRACT_16BITS(msgptr.v4->msg_len);
@@ -616,22 +616,25 @@ olsr_print(netdissect_options *ndo,

case OLSR_NAMESERVICE_MSG:
{
u_int name_entries = EXTRACT_16BITS(msg_data+2);
u_int addr_size = 4;
int name_entries_valid = 0;
u_int name_entries;
u_int addr_size;
int name_entries_valid;
u_int i;

if (msg_tlen < 4)
goto trunc;
ND_TCHECK2(*msg_data, 4);

name_entries = EXTRACT_16BITS(msg_data+2);
addr_size = 4;
if (is_ipv6)
addr_size = 16;

name_entries_valid = 0;
if ((name_entries > 0)
&& ((name_entries * (4 + addr_size)) <= msg_tlen))
name_entries_valid = 1;

if (msg_tlen < 4)
goto trunc;
ND_TCHECK2(*msg_data, 4);

ND_PRINT((ndo, "\n\t Version %u, Entries %u%s",
EXTRACT_16BITS(msg_data),
name_entries, (name_entries_valid == 0) ? " (invalid)" : ""));
@@ -575,6 +575,8 @@ lldp_8023_mtu-oobr lldp_8023_mtu-oobr.pcap lldp_8023_mtu-oobr.out -v -c1
bgp_vpn_rt-oobr bgp_vpn_rt-oobr.pcap bgp_vpn_rt-oobr.out -v -c1
cfm_sender_id-oobr cfm_sender_id-oobr.pcap cfm_sender_id-oobr.out -v -c1
isis-extd-isreach-oobr isis-extd-isreach-oobr.pcap isis-extd-isreach-oobr.out -v -c4
olsr-oobr-1 olsr-oobr-1.pcap olsr-oobr-1.out -v
olsr-oobr-2 olsr-oobr-2.pcap olsr-oobr-2.out -v

# bad packets from Katie Holly
mlppp-oobr mlppp-oobr.pcap mlppp-oobr.out
@@ -0,0 +1,16 @@
IP truncated-ip - 2315 bytes missing! (tos 0x0, ttl 18, id 4111, offset 0, flags [+, DF, rsvd], proto UDP (17), length 5373, bad cksum 8e7f (->9764)!)
15.251.128.192.698 > 193.192.186.0.122: OLSRv4, seq 0x0800, length 2056
Nameservice Message (0x82), originator 126.198.193.192, ttl 26, hop 145
vtime 0.062s, msg-seq 0x0008, length 127[|olsr]

This comment has been minimized.

Copy link
@gvanem

gvanem Sep 14, 2017

Contributor

A slight failure on this test using MSVC. The diff:

--- olsr-oobr-1.out
+++ NEW_py/olsr-oobr-1.out
@@ -2,5 +2,5 @@
     15.251.128.192.698 > 193.192.186.0.122: OLSRv4, seq 0x0800, length 2056
        Nameservice Message (0x82), originator 126.198.193.192, ttl 26, hop 145
-         vtime 0.063s, msg-seq 0x0008, length 127[|olsr]
+         vtime 0.062s, msg-seq 0x0008, length 127[|olsr]
 IP truncated-ip - 2315 bytes missing! (tos 0x0, ttl 18, id 4111, offset 0, flags [+, DF, rsvd], proto UDP (17), length 5373, bad cksum 8e7f (->975f)!)
     16.0.128.192.698 > 193.192.186.0.122: OLSRv4, seq 0x0400, length 512

A whopping 1 msec!

The same for the other vtime values. Caused by the ME_TO_DOUBLE() macro used in print-olsr.c.
Seems this macro ass-u_mes gcc.

Edit: I ran all tests using my Python-script, not the bash/perl stuff (works like a charm).

This comment has been minimized.

Copy link
@infrastation

infrastation Sep 14, 2017

Member

See tests/lmp-v.sh for a very similar case.

This comment has been minimized.

Copy link
@gvanem

gvanem Sep 14, 2017

Contributor

Thanks for the reminder. Old story which seems hard to fix.

This comment has been minimized.

Copy link
@infrastation

infrastation Sep 15, 2017

Member

Gisle, could you tell which CPU your host has and whether MSVC uses FPU for float calculations?

This comment has been minimized.

Copy link
@guyharris

guyharris Sep 15, 2017

Author Member

whether MSVC uses FPU for float calculations?

As in "uses the FPU or uses SSE" - the FPU instructions do 80-bit floating-point internally, with 80-bit registers, but the SSE instructions do purely 64-bit floating-point (as do, I think, floating-point instructions on most other processors).

This comment has been minimized.

Copy link
@gvanem

gvanem Sep 15, 2017

Contributor

@infrastation I have an Intel-i7 CPU. Seems MSVC uses SSE2 for that macro.

double me_to_double (uint8_t vtime)
{
  return ME_TO_DOUBLE (vtime);
}

Disassembles into:

 _me_to_double:
         sub             esp,0x00000008
         mov             dl,byte ptr 0xc[esp]
         mov             eax,0x00000001
         movzx           ecx,dl
         and             ecx,0x0000000f
         shl             eax,cl
         shr             dl,0x04
         movd            xmm2,eax
         movzx           eax,dl
         cvtdq2pd        xmm2,xmm2
         movd            xmm1,eax
         cvtdq2pd        xmm1,xmm1
         mulsd           xmm1,__real@3fb0000000000000
         addsd           xmm1,__real@3ff0000000000000
         mulsd           xmm1,__real@3fb0000000000000
         mulsd           xmm2,xmm1
         movsd           [esp],xmm2
         fld             qword ptr [esp]
         add             esp,0x00000008
         ret

I get the same 1 msec "error" when using -arch:IA32 (no SSE instructions). No surprise there.

This comment has been minimized.

Copy link
@infrastation

infrastation Sep 15, 2017

Member

The current tests in the git repository are produced with GCC's -ffloat-store, which make then reproduce exactly on all architectures. When the compiler is not GCC, those tests are skipped. Perhaps it would help to have an equivalent flag enabled for other compilers if they can reliably implement the same behaviour. Or we can move this test to a GCC-specific block and forget about it for a while longer.

IP truncated-ip - 2315 bytes missing! (tos 0x0, ttl 18, id 4111, offset 0, flags [+, DF, rsvd], proto UDP (17), length 5373, bad cksum 8e7f (->975f)!)
16.0.128.192.698 > 193.192.186.0.122: OLSRv4, seq 0x0400, length 512
Powerinfo Message (0x80), originator 0.1.0.0, ttl 255, hop 255
vtime 0.500s, msg-seq 0x0000, length 9216 (invalid)
IP truncated-ip - 2315 bytes missing! (tos 0x0, ttl 18, id 4111, offset 0, flags [+, DF, rsvd], proto UDP (17), length 5373, bad cksum 8e7f (->9764)!)
15.251.128.192.698 > 193.192.186.0.122: OLSRv4, seq 0x0800, length 2056
Nameservice Message (0x82), originator 126.198.193.192, ttl 26, hop 145
vtime 0.062s, msg-seq 0x0008, length 100[|olsr]
IP truncated-ip - 2315 bytes missing! (tos 0x0, ttl 18, id 4111, offset 0, flags [+, DF, rsvd], proto UDP (17), length 5373, bad cksum 8e7f (->975f)!)
16.0.128.192.698 > 193.192.186.0.122: OLSRv4, seq 0x0800, length 2056
Nameservice Message (0x82), originator 126.198.193.192, ttl 26, hop 145
vtime 0.062s, msg-seq 0x5c50, length 185[|olsr]
BIN +332 Bytes tests/olsr-oobr-1.pcap
Binary file not shown.
@@ -0,0 +1,3 @@
[|ether]
[|ether]
IP6 (flowlabel 0x06400, hlim 0, next-header UDP (17) payload length: 5401) 0:24::1e:a0a:141e.698 > 38fd:7f49:eaff:ffff:2025:7373:7562:2573.2: OLSRv6, seq 0x0201, length 5393[|olsr]
BIN +152 Bytes tests/olsr-oobr-2.pcap
Binary file not shown.

0 comments on commit 0cb1b8a

Please sign in to comment.
You can’t perform that action at this time.