Skip to content

Commit

Permalink
CVE-2017-5483/SNMP: improve ASN.1 bounds checks
Browse files Browse the repository at this point in the history
Kamil Frankowicz had found that truncated BE_STR and BE_SEQ ASN.1
elements could lead to an overread, from the source code it looked like
other ids could have this problem too. Move the checks introduced in
commit 72e501f out of the switch blocks to cover all ids by default.
This fixes GH#559 and GH#566.
  • Loading branch information
infrastation authored and fxlb committed Jan 18, 2017
1 parent c39c1d9 commit eec1624
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 4 deletions.
5 changes: 1 addition & 4 deletions print-snmp.c
Expand Up @@ -519,6 +519,7 @@ asn1_parse(netdissect_options *ndo,
ND_PRINT((ndo, "[id?%c/%s/%d]", *Form[form], Class[class].name, id));
return -1;
}
ND_TCHECK2(*p, elem->asnlen);

switch (form) {
case PRIMITIVE:
Expand All @@ -539,7 +540,6 @@ asn1_parse(netdissect_options *ndo,
ND_PRINT((ndo, "[asnlen=0]"));
return -1;
}
ND_TCHECK2(*p, elem->asnlen);
if (*p & ASN_BIT8) /* negative */
data = -1;
for (i = elem->asnlen; i-- > 0; p++)
Expand Down Expand Up @@ -577,7 +577,6 @@ asn1_parse(netdissect_options *ndo,
case GAUGE:
case TIMETICKS: {
register uint32_t data;
ND_TCHECK2(*p, elem->asnlen);
elem->type = BE_UNS;
data = 0;
for (i = elem->asnlen; i-- > 0; p++)
Expand All @@ -588,7 +587,6 @@ asn1_parse(netdissect_options *ndo,

case COUNTER64: {
register uint64_t data64;
ND_TCHECK2(*p, elem->asnlen);
elem->type = BE_UNS64;
data64 = 0;
for (i = elem->asnlen; i-- > 0; p++)
Expand Down Expand Up @@ -627,7 +625,6 @@ asn1_parse(netdissect_options *ndo,

default:
ND_PRINT((ndo, "[P/%s/%s]", Class[class].name, Class[class].Id[id]));
ND_TCHECK2(*p, elem->asnlen);
elem->type = BE_OCTET;
elem->data.raw = (const uint8_t *)p;
break;
Expand Down
4 changes: 4 additions & 0 deletions tests/TESTLIST
Expand Up @@ -426,6 +426,10 @@ 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
q933-heapoverflow-2 q933-heapoverflow-2.pcap q933-heapoverflow-2.out -t

# bad packets from Kamil Frankowicz
snmp-heapoverflow-1 snmp-heapoverflow-1.pcap snmp-heapoverflow-1.out -t
snmp-heapoverflow-2 snmp-heapoverflow-2.pcap snmp-heapoverflow-2.out -t

# RTP tests
# fuzzed pcap
rtp-seg-fault-1 rtp-seg-fault-1.pcap rtp-seg-fault-1.out -t -v -T rtp
Expand Down
21 changes: 21 additions & 0 deletions tests/snmp-heapoverflow-1.out
@@ -0,0 +1,21 @@
30:30:30:30:30:30 > 30:30:30:30:30:30, ethertype Unknown (0x3030), length 808464432:
0x0000: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000
0x0010: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000
0x0020: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000
0x0030: 3030 00
30:30:30:30:30:30 > 30:30:30:30:30:30, ethertype Unknown (0x3030), length 808464432:
0x0000: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000
0x0010: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000
0x0020: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000
0x0030: 3030 00
30:30:30:30:30:30 > 30:30:30:30:30:30, ethertype Unknown (0x3030), length 808464432:
0x0000: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000
0x0010: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000
0x0020: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000
0x0030: 3030 00
30:30:30:30:30:30 > 30:30:30:30:30:30, ethertype Unknown (0x3030), length 808464432:
0x0000: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000
0x0010: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000
0x0020: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000
0x0030: 3030 00
IP 48.48.48.48.12336 > 48.48.48.48.161: [|snmp]
Binary file added tests/snmp-heapoverflow-1.pcap
Binary file not shown.
1 change: 1 addition & 0 deletions tests/snmp-heapoverflow-2.out
@@ -0,0 +1 @@
IP 48.48.48.48.12336 > 48.48.48.48.162: [|snmp]
Binary file added tests/snmp-heapoverflow-2.pcap
Binary file not shown.

0 comments on commit eec1624

Please sign in to comment.