Skip to content

Commit 3b36ec4

Browse files
committed
CVE-2017-13045/VQP: add some bounds checks
This fixes a buffer over-read discovered by Bhargava Shastry, SecT/TU Berlin. Add a test using the capture file supplied by the reporter(s).
1 parent c2f6833 commit 3b36ec4

File tree

4 files changed

+18
-1
lines changed

4 files changed

+18
-1
lines changed

Diff for: print-vqp.c

+13-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "netdissect.h"
2727
#include "extract.h"
2828
#include "addrtoname.h"
29+
#include "ether.h"
2930

3031
#define VQP_VERSION 1
3132
#define VQP_EXTRACT_VERSION(x) ((x)&0xFF)
@@ -105,13 +106,15 @@ vqp_print(netdissect_options *ndo, register const u_char *pptr, register u_int l
105106
const u_char *tptr;
106107
uint16_t vqp_obj_len;
107108
uint32_t vqp_obj_type;
108-
int tlen;
109+
u_int tlen;
109110
uint8_t nitems;
110111

111112
tptr=pptr;
112113
tlen = len;
113114
vqp_common_header = (const struct vqp_common_header_t *)pptr;
114115
ND_TCHECK(*vqp_common_header);
116+
if (sizeof(struct vqp_common_header_t) > tlen)
117+
goto trunc;
115118

116119
/*
117120
* Sanity checking of the header.
@@ -151,6 +154,9 @@ vqp_print(netdissect_options *ndo, register const u_char *pptr, register u_int l
151154
while (nitems > 0 && tlen > 0) {
152155

153156
vqp_obj_tlv = (const struct vqp_obj_tlv_t *)tptr;
157+
ND_TCHECK(*vqp_obj_tlv);
158+
if (sizeof(struct vqp_obj_tlv_t) > tlen)
159+
goto trunc;
154160
vqp_obj_type = EXTRACT_32BITS(vqp_obj_tlv->obj_type);
155161
vqp_obj_len = EXTRACT_16BITS(vqp_obj_tlv->obj_length);
156162
tptr+=sizeof(struct vqp_obj_tlv_t);
@@ -167,9 +173,13 @@ vqp_print(netdissect_options *ndo, register const u_char *pptr, register u_int l
167173

168174
/* did we capture enough for fully decoding the object ? */
169175
ND_TCHECK2(*tptr, vqp_obj_len);
176+
if (vqp_obj_len > tlen)
177+
goto trunc;
170178

171179
switch(vqp_obj_type) {
172180
case VQP_OBJ_IP_ADDRESS:
181+
if (vqp_obj_len != 4)
182+
goto trunc;
173183
ND_PRINT((ndo, "%s (0x%08x)", ipaddr_string(ndo, tptr), EXTRACT_32BITS(tptr)));
174184
break;
175185
/* those objects have similar semantics - fall through */
@@ -182,6 +192,8 @@ vqp_print(netdissect_options *ndo, register const u_char *pptr, register u_int l
182192
/* those objects have similar semantics - fall through */
183193
case VQP_OBJ_MAC_ADDRESS:
184194
case VQP_OBJ_MAC_NULL:
195+
if (vqp_obj_len != ETHER_ADDR_LEN)
196+
goto trunc;
185197
ND_PRINT((ndo, "%s", etheraddr_string(ndo, tptr)));
186198
break;
187199
default:

Diff for: tests/TESTLIST

+1
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,7 @@ isakmpv1-attr-oobr isakmpv1-attr-oobr.pcap isakmpv1-attr-oobr.out -v
562562
hncp_dhcpv6data-oobr hncp_dhcpv6data-oobr.pcap hncp_dhcpv6data-oobr.out -v -c1
563563
# Same comments apply to the case below.
564564
hncp_dhcpv4data-oobr hncp_dhcpv4data-oobr.pcap hncp_dhcpv4data-oobr.out -v -c1
565+
vqp-oobr vqp-oobr.pcap vqp-oobr.out -v -c1
565566

566567
# bad packets from Katie Holly
567568
mlppp-oobr mlppp-oobr.pcap mlppp-oobr.out

Diff for: tests/vqp-oobr.out

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
IP (tos 0x0, ttl 17, id 40207, offset 0, flags [+, DF, rsvd], proto UDP (17), length 46, bad cksum 8f04 (->f897)!)
2+
0.0.128.20.1589 > 12.251.167.8.62720:
3+
VQPv1, unknown (127) Message, error-code unknown (31) (31), seq 0x80f90000, items 27, length 18
4+
[|VQP]

Diff for: tests/vqp-oobr.pcap

262 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)