OWASP / pysap

pysap is an open source Python library that provides modules for crafting and sending packets using SAP's NI, Diag, Enqueue, Router, MS, SNC, IGS, RFC and HDB protocols.
https://owasp.org/www-project-core-business-application-security/
GNU General Public License v2.0
219 stars 61 forks source link

pysapcar archive creation broken #22

Closed okuuva closed 6 years ago

okuuva commented 6 years ago

When trying to create a new archive file with pysapcar, I get the following traceback:

[I] > pysapcar -cvf ~/bash.sar /usr/local/bin/bash
pysapcar version: 0.1.16
pysapcar: Processing archive '/Users/oula/bash.sar' (version 2.01)
[]
d /usr/local/bin/bash
Traceback (most recent call last):
  File "/Users/oula/venvs/pysap2/bin/pysapcar", line 309, in <module>
    pysapcar.main()
  File "/Users/oula/venvs/pysap2/bin/pysapcar", line 144, in main
    self.append(options, args)
  File "/Users/oula/venvs/pysap2/bin/pysapcar", line 208, in append
    sapcar.write()
  File "/Users/oula/venvs/pysap2/lib/python2.7/site-packages/pysap/SAPCAR.py", line 780, in write
    self.fd.write(str(self._sapcar))
  File "/Users/oula/venvs/pysap2/lib/python2.7/site-packages/scapy/packet.py", line 343, in __str__
    return str(self.build())
  File "/Users/oula/venvs/pysap2/lib/python2.7/site-packages/scapy/packet.py", line 444, in build
    p = self.do_build()
  File "/Users/oula/venvs/pysap2/lib/python2.7/site-packages/scapy/packet.py", line 426, in do_build
    pkt = self.self_build()
  File "/Users/oula/venvs/pysap2/lib/python2.7/site-packages/scapy/packet.py", line 407, in self_build
    p = f.addfield(self, p, val)
  File "/Users/oula/venvs/pysap2/lib/python2.7/site-packages/scapy/fields.py", line 155, in addfield
    return self.fld.addfield(pkt,s,val)
  File "/Users/oula/venvs/pysap2/lib/python2.7/site-packages/scapy/fields.py", line 697, in addfield
    return s + b"".join(raw(v) for v in val)
  File "/Users/oula/venvs/pysap2/lib/python2.7/site-packages/scapy/fields.py", line 697, in <genexpr>
    return s + b"".join(raw(v) for v in val)
  File "/Users/oula/venvs/pysap2/lib/python2.7/site-packages/scapy/compat.py", line 72, in raw
    return x.__bytes__()
  File "/Users/oula/venvs/pysap2/lib/python2.7/site-packages/scapy/packet.py", line 345, in __bytes__
    return self.build()
  File "/Users/oula/venvs/pysap2/lib/python2.7/site-packages/scapy/packet.py", line 444, in build
    p = self.do_build()
  File "/Users/oula/venvs/pysap2/lib/python2.7/site-packages/scapy/packet.py", line 426, in do_build
    pkt = self.self_build()
  File "/Users/oula/venvs/pysap2/lib/python2.7/site-packages/scapy/packet.py", line 407, in self_build
    p = f.addfield(self, p, val)
  File "/Users/oula/venvs/pysap2/lib/python2.7/site-packages/scapy/fields.py", line 80, in addfield
    return s+struct.pack(self.fmt, self.i2m(pkt,val))
  File "/Users/oula/venvs/pysap2/lib/python2.7/site-packages/scapy/fields.py", line 878, in i2m
    f = fld.i2len(pkt, fval)
  File "/Users/oula/venvs/pysap2/lib/python2.7/site-packages/scapy/fields.py", line 504, in i2len
    return len(i)
TypeError: object of type 'NoneType' has no len()

I managed to identify the issue: in SAPCARArchiveFilev200Format there seems to be an unused field user_info. When scapy tries to build package, it tries to calculate length for user_info to be set to user_info_length field. The problem is that user_info is never set and defaults to None so scapy barfs when trying to execute len(None). As 2.01 version inherits the format from 2.00, the problem is present there as well.

This is most likely due to scapy changing behavior for fields with None value at some point but I had the same problem with 0.1.15 release that used scapy 2.3.3.

As those fields do not seem to be used anywhere, I started to wonder if they are remnants of some refactoring or an unfinished feature. In the case of former, I already verified that the script works just fine after deleting those fields from the format definition.

martingalloar commented 6 years ago

Good catch @okuuva!!! Should be fixed now, can you confirm?

I choose not to remove the user_info field because it's included in the specs and it might be used in some archive files.

okuuva commented 6 years ago

Yes, works now, thanks!