scapy icon indicating copy to clipboard operation
scapy copied to clipboard

Tolerate malformed server_name TLS extensions

Open evverx opened this issue 2 years ago • 2 comments

The servernames packet list field can contain "Raw" instances when server_name extenstions are malformed. The "i2repr" method doesn't expect that and ends up throwing exceptions when it can't find the "servername" attribute. This patch addresses that by removing the ServerListField class and relying on PacketListField doing the right thing. Another option would be to use something like

[getattr(p, "servername", p) for p in x]

in the "ServerListField.i2repr" method but I think full server names along with their types and lengths are more helpful.

Fixes:

  File "scapy/packet.py", line 563, in __repr__
    val = f.i2repr(self, fval)
          ^^^^^^^^^^^^^^^^^^^^
  File "scapy/layers/tls/extensions.py", line 180, in i2repr
    res = [p.servername for p in x]
          ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "scapy/layers/tls/extensions.py", line 180, in <listcomp>
    res = [p.servername for p in x]
           ^^^^^^^^^^^^
  File "scapy/packet.py", line 467, in __getattr__
    return self.payload.__getattr__(attr)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "scapy/packet.py", line 465, in __getattr__
    fld, v = self.getfield_and_val(attr)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "scapy/packet.py", line 1788, in getfield_and_val
    raise AttributeError(attr)
AttributeError: servername

evverx avatar May 02 '23 18:05 evverx

Codecov Report

Merging #4001 (f9f2b19) into master (047be32) will decrease coverage by 0.01%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4001      +/-   ##
==========================================
- Coverage   81.93%   81.92%   -0.01%     
==========================================
  Files         328      328              
  Lines       75404    75400       -4     
==========================================
- Hits        61783    61773      -10     
- Misses      13621    13627       +6     
Impacted Files Coverage Δ
scapy/layers/tls/extensions.py 84.22% <ø> (+0.42%) :arrow_up:

... and 8 files with indirect coverage changes

codecov[bot] avatar May 02 '23 19:05 codecov[bot]

TLS_Ext_PrettyPacketList doesn't seem to be compatible with Raw instances conceptually. I'll try to figure out how to make them prettier. This PR just dumps bytes and calls it a day :-)

evverx avatar May 18 '23 13:05 evverx