Skip to content

refactor(protocols): range-check link and SSLv2 makers with _make_uint #2629

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.

Sign up for GitHub

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

Merged
JarryShaw merged 1 commit into from
Nov 2, 2026
Merged

refactor(protocols): range-check link and SSLv2 makers with _make_uint #2629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 8 additions & 15 deletions pcapkit/protocols/application/sslv2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,7 @@ def read(self, length: 'Optional[int]' = None, **kwargs: 'Any') -> 'Data_SSLv2':
elif isinstance(body_schema, Schema_ServerHello):
# NOTE: wire bytes always fit; this refuses a programmatically built
# schema, e.g. ``make(body=Schema_ServerHello(session_id_hit=256))``.
if not 0 <= body_schema.session_id_hit <= 255:
raise ProtocolError(f'SSLv2: SESSION-ID-HIT ({body_schema.session_id_hit}) out '
f'of range for a single octet (0-255)')
self._make_uint(int(body_schema.session_id_hit), bits=8, name='session_id_hit')
body = Data_ServerHello(
# NOTE: kept as the raw octet, not normalised to a strict
# bool -- see ``Data_ServerHello.session_id_hit``.
Expand Down Expand Up @@ -297,8 +295,9 @@ def make(self,
for the former, ``SERVER_HELLO`` for the latter); if ``body``
is of a type this maker does not recognise; if the
resulting record is longer than the chosen header form
allows; or if ``padding`` does not fit the 3-octet header
form's single padding octet (``0``-``255``).
allows; or if ``padding`` is not an :class:`int` (a
:obj:`bool` included) that fits the 3-octet header form's
single padding octet (``0``-``255``).

"""
msg_type_val = cast('Enum_HandshakeType',
Expand Down Expand Up @@ -363,11 +362,8 @@ def make(self,
if not 0 <= record_length <= 0x3FFF:
raise ProtocolError(f'SSLv2: record ({record_length} octet(s)) too long for '
f"the 3-octet header form's 16383-octet limit")
if not 0 <= padding <= 0xFF:
raise ProtocolError(f'SSLv2: padding ({padding}) out of range for the '
f"3-octet header form's single padding octet (0-255)")
indicator = (0x40 if is_escape else 0x00) | (record_length >> 8)
padding_val = padding
padding_val = self._make_uint(padding, bits=8, name='padding')
length_low = record_length & 0xFF

return Schema_SSLv2(
Expand Down Expand Up @@ -498,12 +494,9 @@ def _make_sslv2_server_hello(self,
self._make_index(server_version, server_version_default,
namespace=server_version_namespace,
reversed=server_version_reversed, pack=False))
session_id_hit_val = int(session_id_hit)
if not 0 <= session_id_hit_val <= 255:
# NOTE: ``UInt8Field`` packs mod 256, so ``256`` would silently
# write as ``0x00``, turning a hit into a miss.
raise ProtocolError(f'SSLv2: SESSION-ID-HIT ({session_id_hit_val}) out of range '
f'for a single octet (0-255)')
# NOTE: ``int()`` first, so the documented ``True``/``False`` pass;
# ``UInt8Field`` packs mod 256, so ``256`` would turn a hit into a miss.
session_id_hit_val = self._make_uint(int(session_id_hit), bits=8, name='session_id_hit')
return Schema_ServerHello(
# NOTE: preserved as given, not collapsed to a strict 0/1 --
# see ``Data_ServerHello.session_id_hit``.
Expand Down
22 changes: 8 additions & 14 deletions pcapkit/protocols/link/frad.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,26 +242,20 @@ def make(self,
this package keeps (:issue:`1458`).

"""
if not isinstance(dlci, int) or isinstance(dlci, bool) or not 0 <= dlci <= 0x3FF:
raise ProtocolError(f'{self.alias}: invalid DLCI: {dlci!r}')
for name, bit in (('cr', cr), ('fecn', fecn), ('becn', becn), ('de', de)):
if bit not in (0, 1):
raise ProtocolError(f'{self.alias}: invalid {name}: {bit!r}, must be 0 or 1')
if not isinstance(control, int) or isinstance(control, bool) or not 0 <= control <= 0xFF:
raise ProtocolError(f'{self.alias}: invalid control: {control!r}')
if not isinstance(nlpid, int) or isinstance(nlpid, bool) or not 0 <= nlpid <= 0xFF:
raise ProtocolError(f'{self.alias}: invalid nlpid: {nlpid!r}')
dlci = self._make_uint(dlci, bits=10, name='DLCI')
cr = self._make_uint(cr, bits=1, name='cr')
fecn = self._make_uint(fecn, bits=1, name='fecn')
becn = self._make_uint(becn, bits=1, name='becn')
de = self._make_uint(de, bits=1, name='de')
control = self._make_uint(control, bits=8, name='control')
nlpid = self._make_uint(nlpid, bits=8, name='nlpid')

if nlpid == 0x80:
if not isinstance(snap_oui, (bytes, bytearray)) or len(snap_oui) != 3:
raise ProtocolError(f'{self.alias}: invalid snap_oui: {snap_oui!r}, '
f'must be exactly 3 octets when nlpid is 0x80')
if (not isinstance(snap_pid, int) or isinstance(snap_pid, bool)
or not 0 <= snap_pid <= 0xFFFF):
raise ProtocolError(f'{self.alias}: invalid snap_pid: {snap_pid!r}, '
f'must fit in 16 bits when nlpid is 0x80')
_schema_nlpid, _ext_nlpid = 0, 0x80
_oui, _pid = bytes(snap_oui), snap_pid
_oui, _pid = bytes(snap_oui), self._make_uint(snap_pid, bits=16, name='snap_pid')
else:
if snap_oui is not None or snap_pid is not None:
raise ProtocolError(f'{self.alias}: snap_oui and snap_pid are only meaningful '
Expand Down
23 changes: 8 additions & 15 deletions pcapkit/protocols/link/gre.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,30 +257,23 @@ def make(self,
if reserved1 is not None and not checksum_present:
raise ProtocolError(f'{self.alias}: reserved1 given without checksum')
if checksum_present:
if (not isinstance(checksum, int) or isinstance(checksum, bool)
or not 0 <= checksum <= 0xFFFF):
raise ProtocolError(f'{self.alias}: invalid checksum: {checksum!r}')
checksum = self._make_uint(checksum, bits=16, name='checksum')
if reserved1 is None:
reserved1 = 0
elif (not isinstance(reserved1, int) or isinstance(reserved1, bool)
or not 0 <= reserved1 <= 0xFFFF):
raise ProtocolError(f'{self.alias}: invalid reserved1: {reserved1!r}')
else:
reserved1 = self._make_uint(reserved1, bits=16, name='reserved1')

key_present = key is not None
if key_present and (not isinstance(key, int) or isinstance(key, bool)
or not 0 <= key <= 0xFFFF_FFFF):
raise ProtocolError(f'{self.alias}: invalid key: {key!r}')
if key_present:
key = self._make_uint(key, bits=32, name='key')

sequence_present = sequence_number is not None
if sequence_present and (not isinstance(sequence_number, int)
or isinstance(sequence_number, bool)
or not 0 <= sequence_number <= 0xFFFF_FFFF):
raise ProtocolError(f'{self.alias}: invalid sequence_number: {sequence_number!r}')
if sequence_present:
sequence_number = self._make_uint(sequence_number, bits=32, name='sequence_number')

if reserved & ~0x4FF8:
raise ProtocolError(f'{self.alias}: reserved bits outside 0x4FF8: {reserved:#06x}')
if not isinstance(version, int) or isinstance(version, bool) or not 0 <= version <= 7:
raise ProtocolError(f'{self.alias}: invalid version: {version!r}')
self._make_uint(version, bits=3, name='version')

return Schema_GRE(
flags={
Expand Down
9 changes: 4 additions & 5 deletions pcapkit/protocols/link/loopback.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,13 +242,12 @@ def make(self,
"""
if byteorder not in ('big', 'little'):
raise ProtocolError(f'Loopback: invalid byte order: {byteorder!r}')
# NOTE: ``bool`` is an ``int`` subclass, so without the explicit check
# ``True`` would pack as address family 1 (#2289).
if isinstance(family, bool) or not isinstance(family, int) or not 0 <= family <= 0xFFFF_FFFF:
raise ProtocolError(f'Loopback: invalid address family: {family!r}')
# NOTE: ``_make_uint`` refuses a ``bool``, which would otherwise pack
# as address family 1 (#2289).
family = self._make_uint(family, bits=32, name='address family')

return Schema_Loopback(
family=family.to_bytes(4, byteorder),
family=self._make_pack(family, size=4, lilendian=byteorder == 'little'),
payload=payload,
)

Expand Down
26 changes: 12 additions & 14 deletions pcapkit/protocols/link/netlink.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,25 +290,23 @@ def make(self,
"""
if byteorder not in ('big', 'little'):
raise ProtocolError(f'{self.alias}: invalid byte order: {byteorder!r}')
for name, value in (('nlmsg_type', nlmsg_type), ('nlmsg_flags', nlmsg_flags)):
if not isinstance(value, int) or isinstance(value, bool) or not 0 <= value <= 0xFFFF:
raise ProtocolError(f'{self.alias}: invalid {name}: {value!r}')
for name, value in (('nlmsg_seq', nlmsg_seq), ('nlmsg_pid', nlmsg_pid)):
if not isinstance(value, int) or isinstance(value, bool) or not 0 <= value <= 0xFFFF_FFFF:
raise ProtocolError(f'{self.alias}: invalid {name}: {value!r}')
nlmsg_type = self._make_uint(nlmsg_type, bits=16, name='nlmsg_type')
nlmsg_flags = self._make_uint(nlmsg_flags, bits=16, name='nlmsg_flags')
nlmsg_seq = self._make_uint(nlmsg_seq, bits=32, name='nlmsg_seq')
nlmsg_pid = self._make_uint(nlmsg_pid, bits=32, name='nlmsg_pid')

if nlmsg_len is None:
nlmsg_len = 16 + len(payload)
elif (not isinstance(nlmsg_len, int) or isinstance(nlmsg_len, bool)
or not 0 <= nlmsg_len <= 0xFFFF_FFFF):
raise ProtocolError(f'{self.alias}: invalid nlmsg_len: {nlmsg_len!r}')
else:
nlmsg_len = self._make_uint(nlmsg_len, bits=32, name='nlmsg_len')

lilendian = byteorder == 'little'
return Schema_Netlink(
nlmsg_len=nlmsg_len.to_bytes(4, byteorder),
nlmsg_type=nlmsg_type.to_bytes(2, byteorder),
nlmsg_flags=nlmsg_flags.to_bytes(2, byteorder),
nlmsg_seq=nlmsg_seq.to_bytes(4, byteorder),
nlmsg_pid=nlmsg_pid.to_bytes(4, byteorder),
nlmsg_len=self._make_pack(nlmsg_len, size=4, lilendian=lilendian),
nlmsg_type=self._make_pack(nlmsg_type, size=2, lilendian=lilendian),
nlmsg_flags=self._make_pack(nlmsg_flags, size=2, lilendian=lilendian),
nlmsg_seq=self._make_pack(nlmsg_seq, size=4, lilendian=lilendian),
nlmsg_pid=self._make_pack(nlmsg_pid, size=4, lilendian=lilendian),
payload=payload,
)

Expand Down
49 changes: 30 additions & 19 deletions tests/project/test_rfc_constants_one_home_1739_unit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
reads it, and :mod:`pcapkit.toolkit.pypcapfile`, a reader of several, binds
none of those names at all;
* :class:`TestNotReinlined` -- no module in :data:`SWEPT`, the homes included,
spells a homed value anywhere but in the home's own definition: not as an
spells a homed value anywhere but in the home's own definition, or as the
field width a value helper takes (see :func:`_field_widths`): not as an
operand, a call argument, a subscript, a default, a returned value, a tuple
or table element, a branch of a conditional, nor an assignment at any level.
Each match that means something else is listed in :data:`NOT_A_COPY` or
Expand Down Expand Up @@ -237,9 +238,6 @@
'the IP version number, not the Next Header offset',
('pcapkit/protocols/schema/internet/ipv6.py', 'ptr + 6'):
'the end of a Jumbo Payload option (2 octets of header, 4 of value)',
('pcapkit/protocols/link/frad.py', '0 <= snap_pid <= 65535'):
"the max value of a SNAP PID, FRAD's own 2-octet field (:rfc:`2427#section-3`), "
'not an IP total length',
('pcapkit/protocols/link/linux_sll1.py', 'return 16'):
'the fixed length of the LINUX_SLL (v1) header, not the PCAP record header',
('pcapkit/protocols/link/linux_sll2.py', 'return 20'):
Expand Down Expand Up @@ -374,12 +372,6 @@
('pcapkit/protocols/link/gre.py', 'return 16'):
"GRE's own longest possible header -- the 4-octet base plus all three optional "
'extensions at 4 octets each (:rfc:`2784`/:rfc:`2890`) -- not a PCAP record header',
('pcapkit/protocols/link/gre.py', '0 <= checksum <= 65535'):
"the max value of GRE's own 2-octet Checksum field (:rfc:`2784#section-2.5`), "
'not an IP total length',
('pcapkit/protocols/link/gre.py', '0 <= reserved1 <= 65535'):
"the max value of GRE's own 2-octet Reserved1 field (:rfc:`2784#section-2.6`), "
'not an IP total length',
('pcapkit/protocols/schema/link/gre.py', "{'reserved_2': (4, 9)}"):
"the 9-bit span bits 4-12 leave of RFC 2784's 12-bit Reserved0 once RFC 2890 "
'carves ``K``/``S`` out of it (:rfc:`2890#section-2`), not a PCAP-NG resolution-places '
Expand Down Expand Up @@ -449,17 +441,14 @@
'and the 28-octet IPv4 ARP length'),
'pcapkit/protocols/link/loopback.py': ((24, 28), 'the BSD loopback address families of IPv6 '
'(NetBSD and OpenBSD 24, FreeBSD 28)'),
'pcapkit/protocols/link/ieee802_11.py': ((6, 9, 12, 16, 32), 'a 6-octet MAC address, '
'Frame Control bit offsets, and the 12-, 16- and '
'32-bit widths of 802.11 header fields'),
'pcapkit/protocols/link/ieee802_11.py': ((6, 9, 12), 'a 6-octet MAC address, and '
'Frame Control bit offsets'),
'pcapkit/protocols/link/l2tpv2.py': ((6, 12, 16), 'L2TPv2 header flags, widths and length'),
'pcapkit/protocols/link/linux_sll1.py': ((65535,), "the max value of LINUX_SLL's own "
'2-octet link_addr_length field, not an IP total '
'length'),
'pcapkit/protocols/link/netlink.py': ((16, 65535), "the nlmsghdr fixed header length (16 "
'octets, not a PCAP record header) and the max value '
"of its 16-bit nlmsg_type/nlmsg_flags fields (not "
'an IPv4 total length)'),
'pcapkit/protocols/link/netlink.py': ((16,), 'the nlmsghdr fixed header length (16 '
'octets), not a PCAP record header'),
'pcapkit/protocols/link/radiotap.py': ((65535,), "the max value of radiotap's own "
'2-octet it_len field, not an IP total length'),
'pcapkit/protocols/schema/link/ethernet.py': ((6,), 'a MAC address is 6 octets'),
Expand Down Expand Up @@ -619,14 +608,36 @@ def _context(text: 'str', operand: 'ast.Constant', parents: 'dict[int, ast.AST]'
return _segment(text, node)


#: Value helpers and the keyword each takes a field width by, in bits or octets (#2614).
_WIDTHS = {'_make_uint': 'bits', '_make_sint': 'bits', '_make_pack': 'size',
'_read_uint': 'size', '_make_octets': 'size'}


def _field_widths(tree: 'ast.Module') -> 'set[int]':
"""``id`` of every constant given as a width to a :data:`_WIDTHS` helper.

A field's own width, not a copy of a homed constant that shares its value.

"""
exempt = set() # type: set[int]
for node in ast.walk(tree):
callee = getattr(getattr(node, 'func', None), 'attr', None)
if isinstance(node, ast.Call) and callee in _WIDTHS:
exempt.update(id(keyword.value) for keyword in node.keywords
if keyword.arg == _WIDTHS[callee]
and isinstance(keyword.value, ast.Constant))
return exempt


def _sweep() -> 'list[tuple[str, int, int, str]]':
"""Every homed value in :data:`SWEPT`: module, value, line, expression.

A value is swept wherever it is spelled -- an operand, a call argument, a
subscript, a default, a returned value, an element of a tuple or a table,
a branch of a conditional, or an assignment at any level, which is how a
renamed copy (``_TCP_HDR = 20``) would read. Only the homes' own
definitions, their tables included, are not hits.
definitions, their tables included, and the widths :func:`_field_widths`
finds are not hits.

"""
values = _swept_values()
Expand All @@ -637,7 +648,7 @@ def _sweep() -> 'list[tuple[str, int, int, str]]':
text = path.read_text(encoding='utf-8')
tree = ast.parse(text)
parents = _parents(tree)
sites = _home_sites(rel, tree)
sites = _home_sites(rel, tree) | _field_widths(tree)
for node in ast.walk(tree):
if (isinstance(node, ast.Constant) and type(node.value) is int
and node.value in values and id(node) not in sites):
Expand Down
Loading
Loading