Address DHCP python3 bug

[ujwalkomarla]

First Issue

Fixes #703 for dhcp in Python3.

Test packet data:

packet = b'\xff\xff\xff\xff\xff\xff\x02\x04\x96\x98?\xa0\x81\x00\x03\xfc\x08\x00E\x00\x01C\xc4I@\x00@\x11V\xd3\n\x8b\x14\x03\xff\xff\xff\xff\x00C\x00D\x01/\x17\x99\x02\x01\x06\x00UT2\x10\x00\x00\x80\x00\x00\x00\x00\x00\n\x8b\x14K\x00\x00\x00\x00\x00\x00\x00\x00\x00\x04\x96\xce`\x0e\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00c\x82Sc3\x04\x00\x008@5\x01\x02\x01\x04\xff\xff\xff\x006\x04\n\x8b\x14\x03\x03\x04\n\x8b\x14\x01\x06\x04\x86\x8dO\xc9\x0f\x13extremenetworks.com\xff'

Before:

>>> from impacket import ImpactDecoder
>>> decoder = ImpactDecoder.DHCPDecoder()
>>> d = decoder.decode(packet)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/ukomarla/Downloads/gitRepos/impacket/impacket/ImpactDecoder.py", line 978, in decode
    d = dhcp.DhcpPacket(aBuffer)
  File "/Users/ukomarla/Downloads/gitRepos/impacket/impacket/dhcp.py", line 148, in __init__
    structure.Structure.__init__(self, data, alignment)
  File "/Users/ukomarla/Downloads/gitRepos/impacket/impacket/structure.py", line 87, in __init__
    self.fromString(data)
  File "/Users/ukomarla/Downloads/gitRepos/impacket/impacket/structure.py", line 152, in fromString
    self[field[0]] = self.unpack(field[1], data[:size], dataClassOrCode = dataClassOrCode, field = field[0])
  File "/Users/ukomarla/Downloads/gitRepos/impacket/impacket/structure.py", line 307, in unpack
    return eval(dataClassOrCode, {}, fields)
  File "<string>", line 1, in <module>
  File "/Users/ukomarla/Downloads/gitRepos/impacket/impacket/dhcp.py", line 174, in unpackOptions
    name, format = self.getOptionNameAndFormat(ord(options[i]))
TypeError: ('ord() expected string of length 1, but int found', "When unpacking field 'options | _ | b''[:0]'")

After:

>>> decoder = ImpactDecoder.DHCPDecoder()
>>> d = decoder.decode(packet)
>>> d.fields
{'cookie': 4294967295, '_options': b'\xff\xff\x02\x04\x96\x98?\xa0\x81\x00\x03\xfc\x08\x00E\x00\x01C\xc4I@\x00@\x11V\xd3\n\x8b\x14\x03\xff\xff\xff\xff\x00C\x00D\x01/\x17\x99\x02\x01\x06\x00UT2\x10\x00\x00\x80\x00\x00\x00\x00\x00\n\x8b\x14K\x00\x00\x00\x00\x00\x00\x00\x00\x00\x04\x96\xce`\x0e\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00c\x82Sc3\x04\x00\x008@5\x01\x02\x01\x04\xff\xff\xff\x006\x04\n\x8b\x14\x03\x03\x04\n\x8b\x14\x01\x06\x04\x86\x8dO\xc9\x0f\x13extremenetworks.com\xff', 'options': [('eof', None), ('pad', None), ('pad', None), ('pad', None), ('pad', None), ('pad', None), ('pad', None), ('pad', None), ('pad', None), ('pad', None), ('pad', None), ('pad', None)]}

---

Second Issue:

Sample code to reproduce the issue

>>> from impacket import ImpactPacket
>>> from impacket.dhcp import DhcpPacket, BootpPacket
>>> dhcp_option_list = ["subnet-mask","router","domain-name","domain-name-server","vendor-specific","vendor-class"]
>>>
>>> # DHCP
... d = DhcpPacket()
>>> d["cookie"] = DhcpPacket.MAGIC_NUMBER
>>>
>>> opt = []
>>> opt.append(("message-type", DhcpPacket.DHCPDISCOVER))
>>> reqst = bytearray()
>>> # format DHCP request just like EXOS
... for oname in dhcp_option_list:
...         code, fmt = d.options.get(oname)
...         reqst.extend(chr(code).encode())
...
>>> opt.append(("parameter-request-list", reqst.decode()))
>>> sysName = "XYQ"
>>> opt.append(("vendor-class", sysName))
>>> opt.append(("eof", None))
>>>
>>> d["options"] = opt
>>> d.getData()

Before:
b"c\x82Sc5\x01b'\\x01'7\x06b'\\x01\\x03\\x0f\\x06+<'<\x03b'XYZ'\xff\x00b''"

After:
b'c\x82Sc5\x01\x017\x06\x01\x03\x0f\x06+<<\x03XYZ\xff\x00'

[alexisbalbachan]

This code does resolve the mentioned issues and works fine, however i'm suggesting an alternative which does not rely on six/if statements. It should work fine in both python2 and python3.

[ujwalkomarla]

This code does resolve the mentioned issues and works fine, however i'm suggesting an alternative which does not rely on six/if statements. It should work fine in both python2 and python3.

The second code above throws an error:

Traceback (most recent call last):
  File "/Users/ukomarla/Downloads/gitRepos/uj_impacket/impacket/structure.py", line 202, in pack
    return self.pack(two[0], data)
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ukomarla/Downloads/gitRepos/uj_impacket/impacket/structure.py", line 266, in pack
    raise Exception("Trying to pack None")
Exception: Trying to pack None

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/ukomarla/Downloads/gitRepos/uj_impacket/impacket/structure.py", line 126, in getData
    data += self.packField(field[0], field[1])
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ukomarla/Downloads/gitRepos/uj_impacket/impacket/structure.py", line 113, in packField
    ans = self.pack(format, None, field = fieldName)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ukomarla/Downloads/gitRepos/uj_impacket/impacket/structure.py", line 206, in pack
    return self.pack(two[0], eval(two[1], {}, fields))
                             ^^^^^^^^^^^^^^^^^^^^^^^^
  File "<string>", line 1, in <module>
  File "/Users/ukomarla/Downloads/gitRepos/uj_impacket/impacket/dhcp.py", line 157, in packOptions
    answer += b'%c%c%s' % (code, len(val), val)
TypeError: ('can only concatenate str (not "bytes") to str', "When packing field '_options | :=self.packOptions(options)' in <class 'impacket.dhcp.DhcpPacket'>")

[alexisbalbachan]

Hi @ujwalkomarla. Can you make sure that all the changes were applied when you ran the test?

• answer should be defined as a bytes object :

  impacket/impacket/dhcp.py

  Line 154 in 5721694

  answer = b''

• And we're only concatenating bytes objects to it: answer += b'%c%c%s' % (code, len(val), val)
• The error you encountered seems to indicate that answer is defined as a str object: TypeError: ('can only concatenate str (not "bytes") to str'