diff options
| author | Peter Wu <peter@lekensteyn.nl> | 2017-05-05 11:46:07 +0200 |
|---|---|---|
| committer | Peter Wu <peter@lekensteyn.nl> | 2017-05-11 14:52:44 +0000 |
| commit | 129bdb5a164a6386c35ff387e9d8f0d3d6a12dbf (patch) | |
| tree | 59ff6aa1de70c20dde394ab1c19ea9c92b3df7d1 | |
| parent | 79eab8ca070f978415126f85b0777ab4ab02f0a2 (diff) | |
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>
| -rw-r--r-- | epan/dissectors/packet-dns.c | 16 |
1 files changed, 7 insertions, 9 deletions
diff --git a/epan/dissectors/packet-dns.c b/epan/dissectors/packet-dns.c index 373272ea41..ec02d306ac 100644 --- a/epan/dissectors/packet-dns.c +++ b/epan/dissectors/packet-dns.c @@ -1134,8 +1134,7 @@ expand_dns_name(tvbuff_t *tvb, int offset, int max_len, int dns_data_offset, int start_offset = offset; guchar *np; int len = -1; - int chars_processed = 0; - int data_size = tvb_reported_length_remaining(tvb, dns_data_offset); + int pointers_count = 0; int component_len; int indir_offset; 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) { break; } - chars_processed++; switch (component_len & 0xc0) { case 0x00: @@ -1184,7 +1182,6 @@ expand_dns_name(tvbuff_t *tvb, int offset, int max_len, int dns_data_offset, } component_len--; offset++; - chars_processed++; } 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 + (((component_len & ~0xc0) << 8) | tvb_get_guint8(tvb, offset)); offset++; - chars_processed++; + pointers_count++; /* 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" @@ -1273,10 +1270,11 @@ expand_dns_name(tvbuff_t *tvb, int offset, int max_len, int dns_data_offset, if (len < 0) { 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 - looping. */ - if (chars_processed >= data_size) { + /* + * If we find a pointer to itself, it is a trivial loop. Otherwise if we + * processed a large number of pointers, assume an indirect loop. + */ + if (indir_offset == offset + 2 || pointers_count > MAXDNAME/4) { *name="<Name contains a pointer that loops>"; *name_len = (guint)strlen(*name); if (len < min_len) { |