osmith
/
wireshark
forked from osmocom/wireshark
1
0
Fork 0
Code Pull Requests Releases Wiki Activity

dns: improve loop detection in label decompression 

Previously the number of allowed pointers within a message is equal to
the data in a tvb (16575 in one example). This is still expensive, so
implement an alternative detection mechanism that looks for a direct
self-loop and limits the total pointers to about 256.

Bug: 13633
Change-Id: I803873e24ab170c7ef0b881d3bdc9dfd4014de97
Link: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=1206
Reviewed-on: https://code.wireshark.org/review/21507
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Martin Kaiser <wireshark@kaiser.cx>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
This commit is contained in:
Peter Wu 2017-05-05 11:46:07 +02:00
parent 79eab8ca07
commit 129bdb5a16
1 changed files with 7 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
epan/dissectors/packet-dns.c
View File

@ -1134,8 +1134,7 @@ expand_dns_name(tvbuff_t *tvb, int offset, int max_len, int dns_data_offset,
int start_offset = offset; int start_offset = offset;
guchar *np; guchar *np;
int len = -1; int len = -1;
int chars_processed = 0; int pointers_count = 0;
int data_size = tvb_reported_length_remaining(tvb, dns_data_offset);
int component_len; int component_len;
int indir_offset; int indir_offset;
int maxname; int maxname;
@ -1160,7 +1159,6 @@ expand_dns_name(tvbuff_t *tvb, int offset, int max_len, int dns_data_offset,
if (component_len == 0) { if (component_len == 0) {
break; break;
} }
chars_processed++;
switch (component_len & 0xc0) { switch (component_len & 0xc0) {
case 0x00: case 0x00:
@ -1184,7 +1182,6 @@ expand_dns_name(tvbuff_t *tvb, int offset, int max_len, int dns_data_offset,
} }
component_len--; component_len--;
offset++; offset++;
chars_processed++;
} }
break; break;
@ -1265,7 +1262,7 @@ expand_dns_name(tvbuff_t *tvb, int offset, int max_len, int dns_data_offset,
indir_offset = dns_data_offset + indir_offset = dns_data_offset +
(((component_len & ~0xc0) << 8) | tvb_get_guint8(tvb, offset)); (((component_len & ~0xc0) << 8) | tvb_get_guint8(tvb, offset));
offset++; offset++;
chars_processed++; pointers_count++;
/* If "len" is negative, we are still working on the original name, /* If "len" is negative, we are still working on the original name,
not something pointed to by a pointer, and so we should set "len" not something pointed to by a pointer, and so we should set "len"
@ -1273,10 +1270,11 @@ expand_dns_name(tvbuff_t *tvb, int offset, int max_len, int dns_data_offset,
if (len < 0) { if (len < 0) {
len = offset - start_offset; len = offset - start_offset;
} }
/* If we've looked at every character in the message, this pointer /*
will make us look at some character again, which means we're * If we find a pointer to itself, it is a trivial loop. Otherwise if we
looping. */ * processed a large number of pointers, assume an indirect loop.
if (chars_processed >= data_size) { */
if (indir_offset == offset + 2 || pointers_count > MAXDNAME/4) {
*name="<Name contains a pointer that loops>"; *name="<Name contains a pointer that loops>";
*name_len = (guint)strlen(*name); *name_len = (guint)strlen(*name);
if (len < min_len) { if (len < min_len) {