summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDenis Ovsienko <denis@ovsienko.info>2017-01-12 13:47:50 +0000
committerFrancois-Xavier Le Bail <fx.lebail@yahoo.com>2017-01-18 09:16:41 +0100
commiteec1624f7be88008f519d92150ee0eb85633518b (patch)
tree77656a46eec698c55703affe9be45d6f51c363d8
parentc39c1d99ac3b6d5d9519b39da6717180651650d3 (diff)
downloadtcpdump-eec1624f7be88008f519d92150ee0eb85633518b.tar.gz
CVE-2017-5483/SNMP: improve ASN.1 bounds checks
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.
-rw-r--r--print-snmp.c5
-rw-r--r--tests/TESTLIST4
-rw-r--r--tests/snmp-heapoverflow-1.out21
-rw-r--r--tests/snmp-heapoverflow-1.pcapbin0 -> 2804 bytes
-rw-r--r--tests/snmp-heapoverflow-2.out1
-rw-r--r--tests/snmp-heapoverflow-2.pcapbin0 -> 173 bytes
6 files changed, 27 insertions, 4 deletions
diff --git a/print-snmp.c b/print-snmp.c
index 69b6a13f..1b096dcf 100644
--- a/print-snmp.c
+++ b/print-snmp.c
@@ -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:
@@ -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++)
@@ -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++)
@@ -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++)
@@ -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;
diff --git a/tests/TESTLIST b/tests/TESTLIST
index 91c3b8a7..e8856c01 100644
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -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
diff --git a/tests/snmp-heapoverflow-1.out b/tests/snmp-heapoverflow-1.out
new file mode 100644
index 00000000..b8856074
--- /dev/null
+++ b/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]
diff --git a/tests/snmp-heapoverflow-1.pcap b/tests/snmp-heapoverflow-1.pcap
new file mode 100644
index 00000000..83e57595
--- /dev/null
+++ b/tests/snmp-heapoverflow-1.pcap
Binary files differ
diff --git a/tests/snmp-heapoverflow-2.out b/tests/snmp-heapoverflow-2.out
new file mode 100644
index 00000000..98789159
--- /dev/null
+++ b/tests/snmp-heapoverflow-2.out
@@ -0,0 +1 @@
+IP 48.48.48.48.12336 > 48.48.48.48.162: [|snmp]
diff --git a/tests/snmp-heapoverflow-2.pcap b/tests/snmp-heapoverflow-2.pcap
new file mode 100644
index 00000000..19fd2487
--- /dev/null
+++ b/tests/snmp-heapoverflow-2.pcap
Binary files differ