Skip to content

Commit 13ab8d1

Browse files
guyharrisinfrastation
authored andcommitted
CVE-2017-13013/ARP: Fix printing of ARP protocol addresses.
If the protocol type isn't ETHERTYPE_IP or ETHERTYPE_TRAIL, or if the protocol address length isn't 4, don't print the address as an IPv4 address. 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), modified so the capture file won't be rejected as an invalid capture. Update another test file's tcpdump output to reflect this change.
1 parent 8509ef0 commit 13ab8d1

File tree

5 files changed

+173
-116
lines changed

5 files changed

+173
-116
lines changed

Diff for: print-arp.c

+72-17
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ struct arp_pkthdr {
7878
u_char ar_tha[]; /* target hardware address */
7979
u_char ar_tpa[]; /* target protocol address */
8080
#endif
81-
#define ar_sha(ap) (((const u_char *)((ap)+1))+0)
81+
#define ar_sha(ap) (((const u_char *)((ap)+1))+ 0)
8282
#define ar_spa(ap) (((const u_char *)((ap)+1))+ (ap)->ar_hln)
8383
#define ar_tha(ap) (((const u_char *)((ap)+1))+ (ap)->ar_hln+(ap)->ar_pln)
8484
#define ar_tpa(ap) (((const u_char *)((ap)+1))+2*(ap)->ar_hln+(ap)->ar_pln)
@@ -189,6 +189,30 @@ isnonzero(const u_char *a, size_t len)
189189
return (0);
190190
}
191191

192+
static void
193+
tpaddr_print_ip(netdissect_options *ndo,
194+
const struct arp_pkthdr *ap, u_short pro)
195+
{
196+
if (pro != ETHERTYPE_IP && pro != ETHERTYPE_TRAIL)
197+
ND_PRINT((ndo, "<wrong proto type>"));
198+
else if (PROTO_LEN(ap) != 4)
199+
ND_PRINT((ndo, "<wrong len>"));
200+
else
201+
ND_PRINT((ndo, "%s", ipaddr_string(ndo, TPA(ap))));
202+
}
203+
204+
static void
205+
spaddr_print_ip(netdissect_options *ndo,
206+
const struct arp_pkthdr *ap, u_short pro)
207+
{
208+
if (pro != ETHERTYPE_IP && pro != ETHERTYPE_TRAIL)
209+
ND_PRINT((ndo, "<wrong proto type>"));
210+
else if (PROTO_LEN(ap) != 4)
211+
ND_PRINT((ndo, "<wrong len>"));
212+
else
213+
ND_PRINT((ndo, "%s", ipaddr_string(ndo, SPA(ap))));
214+
}
215+
192216
static void
193217
atmarp_addr_print(netdissect_options *ndo,
194218
const u_char *ha, u_int ha_len, const u_char *srca,
@@ -204,6 +228,30 @@ atmarp_addr_print(netdissect_options *ndo,
204228
}
205229
}
206230

231+
static void
232+
atmarp_tpaddr_print(netdissect_options *ndo,
233+
const struct atmarp_pkthdr *ap, u_short pro)
234+
{
235+
if (pro != ETHERTYPE_IP && pro != ETHERTYPE_TRAIL)
236+
ND_PRINT((ndo, "<wrong proto type>"));
237+
else if (ATMTPROTO_LEN(ap) != 4)
238+
ND_PRINT((ndo, "<wrong tplen>"));
239+
else
240+
ND_PRINT((ndo, "%s", ipaddr_string(ndo, ATMTPA(ap))));
241+
}
242+
243+
static void
244+
atmarp_spaddr_print(netdissect_options *ndo,
245+
const struct atmarp_pkthdr *ap, u_short pro)
246+
{
247+
if (pro != ETHERTYPE_IP && pro != ETHERTYPE_TRAIL)
248+
ND_PRINT((ndo, "<wrong proto type>"));
249+
else if (ATMSPROTO_LEN(ap) != 4)
250+
ND_PRINT((ndo, "<wrong splen>"));
251+
else
252+
ND_PRINT((ndo, "%s", ipaddr_string(ndo, ATMSPA(ap))));
253+
}
254+
207255
static void
208256
atmarp_print(netdissect_options *ndo,
209257
const u_char *bp, u_int length, u_int caplen)
@@ -252,18 +300,21 @@ atmarp_print(netdissect_options *ndo,
252300
switch (op) {
253301

254302
case ARPOP_REQUEST:
255-
ND_PRINT((ndo, "who-has %s", ipaddr_string(ndo, ATMTPA(ap))));
303+
ND_PRINT((ndo, "who-has "));
304+
atmarp_tpaddr_print(ndo, ap, pro);
256305
if (ATMTHRD_LEN(ap) != 0) {
257306
ND_PRINT((ndo, " ("));
258307
atmarp_addr_print(ndo, ATMTHA(ap), ATMTHRD_LEN(ap),
259308
ATMTSA(ap), ATMTSLN(ap));
260309
ND_PRINT((ndo, ")"));
261310
}
262-
ND_PRINT((ndo, "tell %s", ipaddr_string(ndo, ATMSPA(ap))));
311+
ND_PRINT((ndo, " tell "));
312+
atmarp_spaddr_print(ndo, ap, pro);
263313
break;
264314

265315
case ARPOP_REPLY:
266-
ND_PRINT((ndo, "%s is-at ", ipaddr_string(ndo, ATMSPA(ap))));
316+
atmarp_spaddr_print(ndo, ap, pro);
317+
ND_PRINT((ndo, " is-at "));
267318
atmarp_addr_print(ndo, ATMSHA(ap), ATMSHRD_LEN(ap), ATMSSA(ap),
268319
ATMSSLN(ap));
269320
break;
@@ -280,11 +331,13 @@ atmarp_print(netdissect_options *ndo,
280331
case ARPOP_INVREPLY:
281332
atmarp_addr_print(ndo, ATMSHA(ap), ATMSHRD_LEN(ap), ATMSSA(ap),
282333
ATMSSLN(ap));
283-
ND_PRINT((ndo, "at %s", ipaddr_string(ndo, ATMSPA(ap))));
334+
ND_PRINT((ndo, "at "));
335+
atmarp_spaddr_print(ndo, ap, pro);
284336
break;
285337

286338
case ARPOP_NAK:
287-
ND_PRINT((ndo, "for %s", ipaddr_string(ndo, ATMSPA(ap))));
339+
ND_PRINT((ndo, "for "));
340+
atmarp_spaddr_print(ndo, ap, pro);
288341
break;
289342

290343
default:
@@ -332,7 +385,7 @@ arp_print(netdissect_options *ndo,
332385
break;
333386
}
334387

335-
if (!ND_TTEST2(*ar_tpa(ap), PROTO_LEN(ap))) {
388+
if (!ND_TTEST2(*TPA(ap), PROTO_LEN(ap))) {
336389
ND_PRINT((ndo, "%s", tstr));
337390
ND_DEFAULTPRINT((const u_char *)ap, length);
338391
return;
@@ -367,16 +420,18 @@ arp_print(netdissect_options *ndo,
367420
switch (op) {
368421

369422
case ARPOP_REQUEST:
370-
ND_PRINT((ndo, "who-has %s", ipaddr_string(ndo, TPA(ap))));
423+
ND_PRINT((ndo, "who-has "));
424+
tpaddr_print_ip(ndo, ap, pro);
371425
if (isnonzero((const u_char *)THA(ap), HRD_LEN(ap)))
372426
ND_PRINT((ndo, " (%s)",
373427
linkaddr_string(ndo, THA(ap), linkaddr, HRD_LEN(ap))));
374-
ND_PRINT((ndo, " tell %s", ipaddr_string(ndo, SPA(ap))));
428+
ND_PRINT((ndo, " tell "));
429+
spaddr_print_ip(ndo, ap, pro);
375430
break;
376431

377432
case ARPOP_REPLY:
378-
ND_PRINT((ndo, "%s is-at %s",
379-
ipaddr_string(ndo, SPA(ap)),
433+
spaddr_print_ip(ndo, ap, pro);
434+
ND_PRINT((ndo, " is-at %s",
380435
linkaddr_string(ndo, SHA(ap), linkaddr, HRD_LEN(ap))));
381436
break;
382437

@@ -387,9 +442,9 @@ arp_print(netdissect_options *ndo,
387442
break;
388443

389444
case ARPOP_REVREPLY:
390-
ND_PRINT((ndo, "%s at %s",
391-
linkaddr_string(ndo, THA(ap), linkaddr, HRD_LEN(ap)),
392-
ipaddr_string(ndo, TPA(ap))));
445+
ND_PRINT((ndo, "%s at ",
446+
linkaddr_string(ndo, THA(ap), linkaddr, HRD_LEN(ap))));
447+
tpaddr_print_ip(ndo, ap, pro);
393448
break;
394449

395450
case ARPOP_INVREQUEST:
@@ -399,9 +454,9 @@ arp_print(netdissect_options *ndo,
399454
break;
400455

401456
case ARPOP_INVREPLY:
402-
ND_PRINT((ndo,"%s at %s",
403-
linkaddr_string(ndo, SHA(ap), linkaddr, HRD_LEN(ap)),
404-
ipaddr_string(ndo, SPA(ap))));
457+
ND_PRINT((ndo,"%s at ",
458+
linkaddr_string(ndo, SHA(ap), linkaddr, HRD_LEN(ap))));
459+
spaddr_print_ip(ndo, ap, pro);
405460
break;
406461

407462
default:

Diff for: tests/TESTLIST

+1
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,7 @@ pktap-heap-overflow pktap-heap-overflow.pcap pktap-heap-overflow.out -v
510510
# bad packets from Bhargava Shastry
511511
lldp_asan lldp_asan.pcap lldp_asan.out -v
512512
extract_read2_asan extract_read2_asan.pcap extract_read2_asan.out -v
513+
getname_2_read4_asan getname_2_read4_asan.pcap getname_2_read4_asan.out -v
513514

514515
# RTP tests
515516
# fuzzed pcap

0 commit comments

Comments
 (0)