secdev / secdev/scapy

Not enough precision in FixedPointField

Open
#3,748 4 comments 0 reactions 0 assignees View on GitHub

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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.