Not enough precision in FixedPointField
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 12.6k
- Forks
- 2.2k
- Avg merge
- 1d 4h
- Merged PRs (30d)
- 56
Description
The call to normalize: https://github.com/secdev/scapy/blob/a1f999fb1721a64415af75d575cabde19625a6f5/scapy/fields.py#L3189 is rounding too aggressively and some bits are lost in the process.
See this snippet that highlights a non-expected result with Scapy 2.5.0rc1.dev20, commit a1f999fb1721a64415af75d575cabde19625a6f5:
fp = FixedPointField("foo", 0, 64, 32)
hex(fp.any2i(None, fp.i2h(None, 0x6f469178))) == '0x6f469178'
I tried to fix it by using math.ceil(math.log10(pw)) instead of int(math.log10(pw)) but it doesn't seem sufficient. When using math.ceil(math.log10(pw)) + 1 it works as expected. I'm not familiar enough with scapy nor decimal so I'm not sure to understand the rationale behind using decimal to manipulate fixed point precision from binary data (my use case is for NTP's TimeStampField) so my fix is ad hoc; in any case, I can open a PR with this patch if you want.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in scapy/fields.py at the normalize call around line 3189 and reproduce the FixedPointField example from the issue. Inspect how decimal precision is chosen for binary fixed-point values, then verify that the round trip preserves 0x6f469178 and add regression coverage for the demonstrated case.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100