Skip to content

opc_da: fix STDOBJREF IPID and Fack selack field types - #5095

Open
mmadersbacher wants to merge 1 commit into
secdev:masterfrom
mmadersbacher:opc-da-field-types
Open

opc_da: fix STDOBJREF IPID and Fack selack field types#5095
mmadersbacher wants to merge 1 commit into
secdev:masterfrom
mmadersbacher:opc-da-field-types

Conversation

@mmadersbacher

Copy link
Copy Markdown
Contributor

Two fields pass a field class where a packet class is expected.

STDOBJREF.IPID wraps UUIDField in a PacketField, so dissection dies:

>>> STDOBJREF(hex_bytes('0100000002000000112233445566778899aabbccddeeff000102030405060708090a0b0c0d0e0f10'))
TypeError: UUIDField.__init__() missing 1 required positional argument: 'default'

_make_le already special-cases UUIDField, so the field was meant to be used directly. STDOBJREF is little-endian only, hence FORMAT_LE.

OpcDaFack.selack passes IntField to a PacketListField. That one fails silently instead, with selackLen=2 dissecting into a single Raw spanning both entries:

>>> OpcDaFack(hex_bytes('0000000010000010000000080000010002aaaaaaaabbbbbbbb')).selack
[<Raw  load=b'\xaa\xaa\xaa\xaa\xbb\xbb\xbb\xbb' |>]

A list of plain fields is FieldListField, which gives [0xaaaaaaaa, 0xbbbbbbbb]. OpcDaFackLE still builds and dissects.

Validated on Windows with Python 3.13. UTscapy -t test/contrib/opc_da.uts -P "load_contrib('opc_da')" gives 12 passed, 0 failed. The Fack section was a # No example yet placeholder and now holds real cases, plus one for STDOBJREF. flake8 scapy/ and mypy_check.py (win32 and linux) are clean. The full windows.utsc campaign gives 96 failures, the same count and the same files as on master, all unrelated and caused by missing tcpdump, tshark and libpcap here.

The test vectors are hand-built from the field layout, not taken from a capture.

Both fields passed a field class where a packet class is expected.

STDOBJREF wrapped UUIDField in a PacketField, so dissection died in
PacketField.m2i with "UUIDField.__init__() missing 1 required
positional argument: 'default'". _make_le already special-cases
UUIDField, so the field was meant to be used directly.

OpcDaFack passed IntField to a PacketListField, which failed silently:
selackLen=2 dissected into a single Raw covering both entries. A list
of plain fields is FieldListField.

AI-Assisted: yes (Claude Code)
@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.58%. Comparing base (1f87020) to head (d191872).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5095      +/-   ##
==========================================
+ Coverage   79.42%   80.58%   +1.16%     
==========================================
  Files         372      390      +18     
  Lines       96507    96858     +351     
==========================================
+ Hits        76652    78057    +1405     
+ Misses      19855    18801    -1054     
Files with missing lines Coverage Δ
scapy/contrib/opc_da.py 84.19% <ø> (+0.39%) ⬆️

... and 303 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Successfully merging this pull request may close these issues.

2 participants