From 129bdb5a164a6386c35ff387e9d8f0d3d6a12dbf Mon Sep 17 00:00:00 2001 From: Peter Wu Date: Fri, 5 May 2017 11:46:07 +0200 Subject: [PATCH] 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 Tested-by: Petri Dish Buildbot Reviewed-by: Martin Kaiser Reviewed-by: Peter Wu --- epan/dissectors/packet-dns.c | 16 +++++++--------- 1 file 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_len = (guint)strlen(*name); if (len < min_len) {