Cleanup DNS decompression + fix warning messages being shown

scapy/layers/dns.py

@@ -29,63 +29,79 @@
 from scapy.pton_ntop import inet_ntop, inet_pton
 
 
-def dns_get_str(s, p, pkt=None, _internal=False):
-    """This function decompresses a string s, from the character p.
-    params:
-     - s: the string to decompress
-     - p: start index of the string
-     - pkt: (optional) an InheritOriginDNSStrPacket packet
-
-    returns: (decoded_string, end_index, left_string)
+def dns_get_str(s, pointer=0, pkt=None, _fullpacket=False):
+    """This function decompresses a string s, starting
+    from the given pointer.
+
+    :param s: the string to decompress
+    :param pointer: first pointer on the string (default: 0)
+    :param pkt: (optional) an InheritOriginDNSStrPacket packet
+
+    :returns: (decoded_string, end_index, left_string)
     """
-    # The _internal parameter is reserved for scapy. It indicates
+    # The _fullpacket parameter is reserved for scapy. It indicates
     # that the string provided is the full dns packet, and thus
     # will be the same than pkt._orig_str. The "Cannot decompress"
     # error will not be prompted if True.
     max_length = len(s)
-    name = b""  # The result = the extracted name
-    burned = 0  # The "burned" data, used to determine the remaining bytes
-    q = None  # Will contain the index after the pointer, to be returned
-    processed_pointers = [p]  # Used to check for decompression loops
+    # The result = the extracted name
+    name = b""
+    # Will contain the index after the pointer, to be returned
+    after_pointer = None
+    processed_pointers = []  # Used to check for decompression loops
+    # Analyse given pkt
+    if pkt and hasattr(pkt, "_orig_s") and pkt._orig_s:
+        s_full = pkt._orig_s
+    else:
+        s_full = None
+    bytes_left = None
     while True:
-        if abs(p) >= max_length:
-            warning("DNS RR prematured end (ofs=%i, len=%i)" % (p, len(s)))
+        if abs(pointer) >= max_length:
+            warning("DNS RR prematured end (ofs=%i, len=%i)" % (pointer,
+                                                                len(s)))
             break
-        cur = orb(s[p])  # current value of the string at p
-        p += 1  # p is now pointing to the value of the pointer
-        burned += 1
+        cur = orb(s[pointer])  # get pointer value
+        pointer += 1  # make pointer go forward
         if cur & 0xc0:  # Label pointer
-            if q is None:
-                # p will follow the pointer, whereas q will not
-                q = p + 1
-            if p >= max_length:
-                warning("DNS incomplete jump token at (ofs=%i)" % p)
+            if after_pointer is None:
+                # after_pointer points to where the remaining bytes start,
+                # as pointer will follow the jump token
+                after_pointer = pointer + 1
+            if pointer >= max_length:
+                warning("DNS incomplete jump token at (ofs=%i)" % pointer)
                 break
-            p = ((cur & ~0xc0) << 8) + orb(s[p]) - 12  # Follow the pointer
-            burned += 1
-            if p in processed_pointers:
+            # Follow the pointer
+            pointer = ((cur & ~0xc0) << 8) + orb(s[pointer]) - 12
+            if pointer in processed_pointers:
                 warning("DNS decompression loop detected")
                 break
-            if pkt and hasattr(pkt, "_orig_s") and pkt._orig_s:
-                name += dns_get_str(pkt._orig_s, p, None, _internal=True)[0]
-                if burned == max_length:
-                    break
-            elif not _internal:
-                raise Scapy_Exception("DNS message can't be compressed" +
-                                      "at this point!")
-            processed_pointers.append(p)
+            if not _fullpacket:
+                # Do we have access to the whole packet ?
+                if s_full:
+                    # Yes -> use it to continue
+                    bytes_left = s[after_pointer:]
+                    s = s_full
+                    max_length = len(s)
+                    _fullpacket = True
+                else:
+                    # No -> abort
+                    raise Scapy_Exception("DNS message can't be compressed" +
+                                          "at this point!")
+            processed_pointers.append(pointer)
             continue
         elif cur > 0:  # Label
-            name += s[p:p + cur] + b"."
-            p += cur
-            burned += cur
+            # cur = length of the string
+            name += s[pointer:pointer + cur] + b"."
+            pointer += cur
         else:
             break
-    if q is not None:
+    if after_pointer is not None:
         # Return the real end index (not the one we followed)
-        p = q
+        pointer = after_pointer
+    if bytes_left is None:
+        bytes_left = s[pointer:]
     # name, end_index, remaining
-    return name, p, s[burned:]
+    return name, pointer, bytes_left
 
 
 def DNSgetstr(*args, **kwargs):
@@ -210,7 +226,6 @@ def i2m(self, pkt, x):
     def getfield(self, pkt, s):
         # Decode the compressed DNS message
         decoded, index, left = dns_get_str(s, 0, pkt)
-        # returns (left, decoded)
         return left, decoded
 
 
@@ -260,7 +275,7 @@ def decodeRR(self, name, s, p):
         p += 10
         rr = DNSRR(b"\x00" + ret + s[p:p + rdlen], _orig_s=s, _orig_p=p)
         if type in [2, 3, 4, 5]:
-            rr.rdata = dns_get_str(s, p, _internal=True)[0]
+            rr.rdata = dns_get_str(s, p, _fullpacket=True)[0]
             del(rr.rdlen)
         elif type in DNSRR_DISPATCHER:
             rr = DNSRR_DISPATCHER[type](b"\x00" + ret + s[p:p + rdlen], _orig_s=s, _orig_p=p)  # noqa: E501
@@ -283,7 +298,7 @@ def getfield(self, pkt, s):
             return s, b""
         while c:
             c -= 1
-            name, p, _ = dns_get_str(s, p, _internal=True)
+            name, p, _ = dns_get_str(s, p, _fullpacket=True)
             rr, p = self.decodeRR(name, s, p)
             if ret is None:
                 ret = rr
@@ -312,13 +327,7 @@ def m2i(self, pkt, s):
         if pkt.type == 1:  # A
             family = socket.AF_INET
         elif pkt.type in [2, 5, 12]:  # NS, CNAME, PTR
-            if hasattr(pkt, "_orig_s") and pkt._orig_s:
-                if orb(s[0]) & 0xc0:
-                    s = dns_get_str(s, 0, pkt)[0]
-                else:
-                    s = dns_get_str(pkt._orig_s, pkt._orig_p, _internal=True)[0]  # noqa: E501
-            else:
-                s = dns_get_str(s, 0)[0]
+            s = dns_get_str(s, 0, pkt)[0]
         elif pkt.type == 16:  # TXT
             ret_s = list()
             tmp_s = s

scapy/sendrecv.py

@@ -44,6 +44,7 @@ class debug:
     recv = []
     sent = []
     match = []
+    crashed_on = None
 
 
 ####################
@@ -860,8 +861,8 @@ def sniff(count=0, store=True, offline=None, prn=None, lfilter=None,
                 try:
                     p = s.recv()
                 except socket.error as ex:
-                    log_runtime.warning("Socket %s failed with '%s' and thus"
-                                        " will be ignored" % (s, ex))
+                    warning("Socket %s failed with '%s' and thus"
+                            " will be ignored" % (s, ex))
                     del sniff_sockets[s]
                     continue
                 except read_allowed_exceptions:

scapy/supersocket.py

@@ -63,6 +63,8 @@ def recv(self, x=MTU):
             raise
         except Exception:
             if conf.debug_dissector:
+                from scapy.sendrecv import debug
+                debug.crashed_on = (cls, val)
                 raise
             pkt = conf.raw_layer(val)
         pkt.time = ts

scapy/tools/UTscapy.py

@@ -28,7 +28,7 @@
 from scapy.consts import WINDOWS
 import scapy.modules.six as six
 from scapy.modules.six.moves import range
-from scapy.compat import base64_bytes
+from scapy.compat import base64_bytes, bytes_hex, plain_str
 
 
 #   Util class   #
@@ -451,6 +451,13 @@ def run_test(test, get_interactive_session, verb=3, ignore_globals=None):
         test.output += "UTscapy: Error during result interpretation:\n"
         test.output += "".join(traceback.format_exception(sys.exc_info()[0], sys.exc_info()[1], sys.exc_info()[2],))
     finally:
+        if test.result == "failed":
+            from scapy.sendrecv import debug
+            # Add optional debugging data to log
+            if debug.crashed_on:
+                cls, val = debug.crashed_on
+                test.output += "\n\nPACKET DISSECTION FAILED ON:\n %s(hex_bytes('%s'))" % (cls.__name__, plain_str(bytes_hex(val)))
+                debug.crashed_on = None
         test.decode()
         if verb > 1:
             print("%(result)6s %(crc)s %(name)s" % test, file=sys.stderr)

test/regression.uts

@@ -6994,23 +6994,22 @@ assert raw(recompressed) == raw(pkt)
 = Advanced dns_get_str tests
 ~ dns
 
-assert dns_get_str(b"\x06cheese\x00blobofdata....\x06hamand\xc0\x0c", 22, _internal=True)[0] == b'hamand.cheese.'
+assert dns_get_str(b"\x06cheese\x00blobofdata....\x06hamand\xc0\x0c", 22, _fullpacket=True)[0] == b'hamand.cheese.'
 
-# This non-regression test is meaningless. Should use real DNS payload
-# in non-regression tests.
-#from scapy.tools.UTscapy import Bunch
-#assert dns_get_str(b"\x06hamand\xc0\x0c", 0, pkt=Bunch(_orig_s=b"\x06cheese\x00blobofdata", _orig_p=22))[0] == b'hamand.cheese.'
+compressed_pkt =  b'\x01\x00^\x00\x00\xfb\xa0\x10\x81\xd9\xd3y\x08\x00E\x00\x01\x14\\\n@\x00\xff\x116n\xc0\xa8F\xbc\xe0\x00\x00\xfb\x14\xe9\x14\xe9\x01\x00Ho\x00\x00\x84\x00\x00\x00\x00\x04\x00\x00\x00\x03\x03188\x0270\x03168\x03192\x07in-addr\x04arpa\x00\x00\x0c\x80\x01\x00\x00\x00x\x00\x0f\x07Android\x05local\x00\x019\x017\x013\x01D\x019\x01D\x01E\x01F\x01F\x01F\x011\x018\x010\x011\x012\x01A\x010\x010\x010\x010\x010\x010\x010\x010\x010\x010\x010\x010\x010\x018\x01E\x01F\x03ip6\xc0#\x00\x0c\x80\x01\x00\x00\x00x\x00\x02\xc03\xc03\x00\x01\x80\x01\x00\x00\x00x\x00\x04\xc0\xa8F\xbc\xc03\x00\x1c\x80\x01\x00\x00\x00x\x00\x10\xfe\x80\x00\x00\x00\x00\x00\x00\xa2\x10\x81\xff\xfe\xd9\xd3y\xc0\x0c\x00/\x80\x01\x00\x00\x00x\x00\x06\xc0\x0c\x00\x02\x00\x08\xc0B\x00/\x80\x01\x00\x00\x00x\x00\x06\xc0B\x00\x02\x00\x08\xc03\x00/\x80\x01\x00\x00\x00x\x00\x08\xc03\x00\x04@\x00\x00\x08'
+
+Ether(compressed_pkt)
 
 = Decompression loop in dns_get_str
 ~ dns
 
-assert dns_get_str(b"\x04data\xc0\x0c", 0, _internal=True)[0] == b"data."
+assert dns_get_str(b"\x04data\xc0\x0c", 0, _fullpacket=True)[0] == b"data.data."
 
 = Prematured end in dns_get_str
 ~ dns
 
-assert dns_get_str(b"\x06da", 0, _internal=True)[0] == b"da."
-assert dns_get_str(b"\x04data\xc0\x01", 0, _internal=True)[0] == b"data."
+assert dns_get_str(b"\x06da", 0, _fullpacket=True)[0] == b"da."
+assert dns_get_str(b"\x04data\xc0\x01", 0, _fullpacket=True)[0] == b"data."
 
 = Other decompression loop in dns_get_str
 ~ dns