Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StrFixedLenField trailing null stripping #437

Closed
X-Cli opened this issue Jan 2, 2017 · 5 comments
Closed

StrFixedLenField trailing null stripping #437

X-Cli opened this issue Jan 2, 2017 · 5 comments

Comments

@X-Cli
Copy link
Contributor

X-Cli commented Jan 2, 2017

Hey,

I am currently writing a packet that contains a padding field, in the middle of a list of fields. This padding length is defined by an arithmetic expression w.r.t. the other fields own sizes and values.

After considering the various options, I thought that a StrFixedLenField was the best fit. StrField is used by the Raw Packet to store arbitrary data, but StrField consumes all remaining data, unless a non-null remain value is defined. In my case, I can compute the size of the padding, and using a remain value equal to the size of the remaining fields seems clumsy.

Unfortunately, when the padding is null-terminated or composed of null-bytes, a call to show() displays mangled packets where the padding trailing null bytes are stripped.
I traced the problem to the following excerpt:

https://github.com/secdev/scapy/blob/master/scapy/fields.py#L503

if type(v) is str:
            v = v.rstrip("\0")

The same display-only alteration is also performed here:
https://github.com/secdev/scapy/blob/master/scapy/fields.py#L524

r = v.rstrip("\0")

Could you explain me the rationale behing this behavior? I can't find any test that breaks when commenting these lines, which otherwise prevent the usage of StrFixedLenField as a "Raw"-like field of fixed size.

Easiest way to reproduce the "bug" is:

from scapy.all import *

class TestPacket(Packet):
  name = 'TestPacket'
  fields_desc = [ StrFixedLenField('data', '', length_from=lambda x: 4)]

TestPacket("tototiti").show()
TestPacket("\Foo\x00"*2).show()
TestPacket("\x00"*8).show()

Thanks.

Cheers,
Florian

@p-l-
Copy link
Member

p-l- commented Jan 17, 2017

That's not a bug. That's how we (often) want fixed-length strings to be represented, since they would be zero-padded, and the padding is not interesting.

@X-Cli
Copy link
Contributor Author

X-Cli commented Jan 17, 2017

Ok. Would you prefer if I wrote a RawFixedLenField in fields.py or within my (contrib) module?

@mwWZ2NYF
Copy link
Contributor

@X-Cli Just in case, have a look at this PR: #446 . The XStrField / XStrLenField / XStrFixedLenField may help you.

@X-Cli
Copy link
Contributor Author

X-Cli commented Jan 17, 2017

@plorinquer Excellent! I certainly could use these fields, once they are merged. Thanks!

@gpotter2
Copy link
Member

@p-l- This Issue may be closed too :)

@guedou guedou closed this as completed Apr 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants