From a1f87ebcd2b8fa8a47ea553961c6a0de44940ce3 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 22 Sep 2026 23:48:57 -0400 Subject: [PATCH] fix(pcapng): stop a negative length reaching a read on a truncated capture An EOF-truncated PCAP-NG file raised a bare `ValueError` out of `Extractor`, losing the whole extraction rather than the one truncated block. - `PCAPNG.read` clamps the post-block seek to the octets the file actually held, warning when Block Total Length overran it. The declared length is cross-checked only against its own trailing copy, so it used to seek past the end -- legal and silent -- and every later block then measured a negative remainder. - `PCAPNG._check_block_floor` reports a tail under twelve octets as the quiet `StreamEOFError` the frame loop already catches, instead of padding a block out of nothing. - `nonnegative()` floors every computed length in the schema at zero, and `bounded_option`/`bounded_area` compose it. A negative reached a `struct` template as `'-8s'` or `read()` as a deficit; neither exception was one of `pcapkit.utilities.exceptions`. - `SystemdJournalExportBlock.post_process` no longer leaks three bare exceptions of the same family: a `struct.error` from unpacking a 64-bit length out of a short buffer, which its own NUL padding reached on any entry of unaligned length; an `OverflowError` from a length at or above `2**63` reaching `BytesIO.read`; and a `UnicodeDecodeError` from a field name, key or value that is not UTF-8. - `Option.register` reports a displaced option schema as a `RegistryWarning`, the last of five unguarded registrars. Measured over all 1,509 octet boundaries of `examples/captures/dhcp.pcapng`: 6 parsed before, 1,495 after, and no level raises from outside the library. Coverage on the two files is unchanged at 99.93%. Fixes #678 --- pcapkit/protocols/misc/pcapng.py | 92 ++- pcapkit/protocols/schema/misc/pcapng.py | 320 +++++++- tests/protocols/misc/test_pcapng_unit.py | 881 ++++++++++++++++++++++- 3 files changed, 1255 insertions(+), 38 deletions(-) diff --git a/pcapkit/protocols/misc/pcapng.py b/pcapkit/protocols/misc/pcapng.py index e69bfedd6..1e57a7b12 100644 --- a/pcapkit/protocols/misc/pcapng.py +++ b/pcapkit/protocols/misc/pcapng.py @@ -176,7 +176,7 @@ from pcapkit.protocols.schema.misc.pcapng import ZigBeeNWKKey as Schema_ZigBeeNWKKey from pcapkit.protocols.schema.schema import Schema from pcapkit.utilities.compat import StrEnum, localcontext -from pcapkit.utilities.exceptions import (FormatError, ProtocolError, RegistryError, +from pcapkit.utilities.exceptions import (FormatError, ProtocolError, RegistryError, StreamEOFError, UnsupportedCall, stacklevel) from pcapkit.utilities.warnings import (AttributeWarning, DeprecatedFormatWarning, ProtocolWarning, RegistryWarning, warn) @@ -985,6 +985,13 @@ def unpack(self, length: 'Optional[int]' = None, **kwargs: 'Any') -> 'Data_PCAPN if cast('Optional[Schema_PCAPNG]', self.__header__) is None: packet = kwargs.get('__packet__', {}) # packet data + # NOTE: reached on the parsing path only, and not by a flag: the + # construction path sets ``__header__`` from ``make`` while packing + # its own buffer, so the branch above is already false by the time it + # gets here. An explicit ``_read`` guard would have been a branch + # that could never be taken. + self._check_block_floor(length) + if self._ctx is not None: self._byte = self._ctx.section.byteorder packet['byteorder'] = self._byte @@ -1065,6 +1072,29 @@ def read(self, length: 'Optional[int]' = None, *, _read: 'bool' = True, #: bytes: Raw block data. self._data = self._read_fileng(schema.block.length) + # NOTE: Block Total Length is cross-checked against its own trailing + # copy and never against the file, so on a capture cut short it runs + # past the real end -- and seeking past the end is legal and silent. + # The *next* block read then measures a negative remainder, since + # ``pcapkit.utilities.decorators.prepare`` derives it as the end of + # the stream less the current position, and a negative ``__length__`` + # is what ``pcapng_block_selector`` hands to + # :class:`~pcapkit.corekit.fields.misc.SchemaField` for the bare + # ``ValueError: read length must be non-negative or -1`` of `#678 + # `__. That cost + # the whole extraction rather than the one truncated block, which is + # the `#431 `__ + # accommodation exactly inverted. ``_read_fileng`` already stopped at + # the end of the file, so the octets it returned are the authority on + # where the block really finishes. + read = len(self._data) + if read < schema.block.length: + warn(f'PCAP-NG: [Block {schema.type}] block length ' + f'{schema.block.length} exceeds the {read} octet(s) left in ' + f'the file; block truncated', ProtocolWarning, + stacklevel=stacklevel()) + seek_cur = min(seek_cur, _seek_set + read) + # move backward to the beginning of next block self._file.seek(seek_cur, io.SEEK_SET) @@ -1208,6 +1238,66 @@ def __index__(self: 'Optional[PCAPNG]' = None) -> 'int': # type: ignore[overrid # Utilities. ########################################################################## + def _check_block_floor(self, length: 'Optional[int]') -> 'None': + """Reject a tail too short to hold any block at all. + + Args: + length: Length of packet data, as the caller declared it, or + :obj:`None` to measure what is left in + :attr:`self._file `. + + Raises: + StreamEOFError: If fewer than twelve octets are left to read. + + A PCAP-NG block is twelve octets at its smallest -- Block Type, Block + Total Length and the trailing copy of it, over an empty body -- which is + what :meth:`__length_hint__` reports. A tail shorter than that is not a + block, so parsing one out of it can only invent fields from + :meth:`FieldBase.unpack `'s + zero padding, and once the four octets of :attr:`PCAPNG.type + ` have been padded out + of nothing ``__length__`` is negative -- which + :func:`~pcapkit.protocols.schema.misc.pcapng.pcapng_block_selector` hands + to :class:`~pcapkit.corekit.fields.misc.SchemaField`, where + :meth:`io.RawIOBase.read` raises the bare ``ValueError`` of `#678 + `__. + + Reporting the end of the stream instead is what keeps the `#431 + `__ accommodation: + :exc:`~pcapkit.utilities.exceptions.StreamEOFError` is an + :exc:`EOFError`, which :meth:`Extractor.record_frames + ` already catches + to stop with the frames read so far, so -- unlike a refusal inside + :meth:`FieldBase.unpack `, + which has no catch point above it and would abort the whole extraction -- + this costs no frame that was actually in the file. It is the same signal + :func:`~pcapkit.utilities.decorators.prepare` raises for a *derived* + remainder of exactly zero; only one, two or three stray octets fell + through it. + + Note: + This runs on the parsing path only, since :meth:`unpack` reaches it + only when no schema has been built yet and the construction path + builds one from :meth:`make` while packing its own buffer. That is + what it should do: a constructed block short enough to trip this floor + is already reported by :meth:`read`'s own Block Total Length check, + which names the length and so says more than an end-of-stream would. + + Measuring an undeclared length needs the stream to seek, and nothing + guards that here because :meth:`__post_init__` has already called + :meth:`~io.IOBase.tell` on it to record ``_seek_set``: a stream that + could not seek never reaches this method. + + """ + if length is None: + current = self._file.tell() + length = self._file.seek(0, io.SEEK_END) - current + self._file.seek(current, io.SEEK_SET) + + if length < 12: + raise StreamEOFError(f'PCAP-NG: block truncated: {length} octet(s) left, ' + 'fewer than the 12 a block needs', quiet=True) + def _get_payload(self) -> 'bytes': """Get payload of :attr:`self.__header__ `. diff --git a/pcapkit/protocols/schema/misc/pcapng.py b/pcapkit/protocols/schema/misc/pcapng.py index 56454f409..99de4fea1 100644 --- a/pcapkit/protocols/schema/misc/pcapng.py +++ b/pcapkit/protocols/schema/misc/pcapng.py @@ -29,7 +29,7 @@ from pcapkit.protocols.schema.schema import EnumSchema, Schema, schema_final from pcapkit.utilities.exceptions import FieldValueError, ProtocolError, stacklevel from pcapkit.utilities.logging import SPHINX_TYPE_CHECKING -from pcapkit.utilities.warnings import ProtocolWarning, SchemaWarning, warn +from pcapkit.utilities.warnings import ProtocolWarning, RegistryWarning, SchemaWarning, warn __all__ = [ 'PCAPNG', @@ -207,6 +207,89 @@ def shb_byteorder_callback(field: 'NumberField', packet: 'dict[str, Any]') -> 'N packet['byteorder'] = field._byteorder +def nonnegative(length: 'Callable[[dict[str, Any]], int]') -> 'Callable[[dict[str, Any]], int]': + """Floor a computed field length at zero. + + Args: + length: Callback computing a field's length from the framing a block, + option or record declares. + + Returns: + A callback returning that length, never below zero. + + Every span in this module is a subtraction -- a block's own Block Total + Length less its fixed fields, an option's declared length less the part of + itself it describes, an option area's leftover -- and every operand of those + subtractions is a wire field that a malformed or truncated capture is free to + set to anything. Nothing made the difference non-negative, and the field + layer does not do it either: :meth:`_TextField.__call__ + ` builds its + :mod:`struct` template as ``f'{length}s'`` unconditionally, so a negative + length becomes the format ``'-8s'`` and :func:`struct.calcsize` raises a bare + :exc:`struct.error`; a negative :class:`~pcapkit.corekit.fields.misc.SchemaField` + length reaches :meth:`io.RawIOBase.read` and raises a bare :exc:`ValueError`. + Neither is one of :mod:`pcapkit.utilities.exceptions`, so a caller cannot tell + either from a bug in its own code, and neither is an :exc:`EOFError`, so + neither is caught by the frame loop -- one malformed block therefore cost the + whole extraction. See `#678 + `__. + + Two shapes reach here. A Block Total Length below the block's own fixed-field + floor -- 28 octets for a Section Header Block, 20 for an Interface + Description Block, 16 for a Custom Block -- makes the area negative directly. + And ``__option_padding__``, which :meth:`OptionField.unpack + ` reports as the part + of a declared area its options did not consume, goes negative when they + consumed *more* than the area held: it subtracts each parsed option's real + size from the area without checking that it fits. Both mean the same thing + for a read -- there are no octets here -- and zero says that, where a + negative says something :mod:`struct` cannot express. + + Clamping rather than refusing is the choice :func:`bounded_option` and + :func:`bounded_area` already made, for the reason their docstrings give: a + block read has no catch point above :meth:`FieldBase.unpack + `, so one refusal aborts the + whole extraction rather than one block, which is what the `#431 + `__ accommodation exists + to prevent. The end of the file is the one case that is *not* a clamp, since + there no block is being read at all -- see :meth:`PCAPNG._check_block_floor + `, which reports it + as the :exc:`~pcapkit.utilities.exceptions.StreamEOFError` the frame loop + catches. + + Note: + Unlike :func:`bounded_option` this needs no ``__length__`` opt-out for + the packing path, because it floors a *difference* rather than clamping + against the remaining area: a negative difference is not a legitimate + thing to pack either -- ``struct.pack('-8s', ...)`` raises exactly as + ``calcsize`` does -- where clamping against the remainder would have + shortened a perfectly good option. + + That distinction is what keeps the three decryption-secrets payloads -- + :attr:`UnknownSecrets.data`, :attr:`TLSKeyLog.data` and + :attr:`WireGuardKeyLog.data` -- out of this. They read ``__length__`` + *whole* rather than subtracting from it, and ``Schema.pack`` leaves it at + ``-1`` for "unknown", so flooring them packs nothing at all. Measured: it + emptied both secrets payloads, and the two ``EXPECTED_FAILURES`` entries + recording their round-trip mismatch then came back ``OK``, since an empty + payload compares equal to an empty payload. On the parsing path their + ``__length__`` is the length the enclosing + :class:`~pcapkit.corekit.fields.misc.SchemaField` declared from a 32-bit + ``secrets_length``, which cannot be negative, so there is nothing there to + floor. + + """ + def callback(pkt: 'dict[str, Any]') -> 'int': + nominal = length(pkt) + if nominal >= 0: + return nominal + + warn(f'PCAP-NG: computed field length is negative ({nominal}); reading 0', + SchemaWarning, stacklevel=stacklevel()) + return 0 + return callback + + def bounded_option(length: 'Callable[[dict[str, Any]], int]') -> 'Callable[[dict[str, Any]], int]': """Clamp an option or record payload to the octets its area still declares. @@ -270,9 +353,18 @@ def bounded_option(length: 'Callable[[dict[str, Any]], int]') -> 'Callable[[dict silently. A clamp applied while packing would therefore shorten a perfectly good option rather than reject it. + The nominal length still goes through :func:`nonnegative` first, on both + paths, since an option declaring less payload than the part of itself it + describes -- ``length`` below the four octets of an ``epb_hash`` or an + ``ns_dnsIP4addr`` record's own fields -- makes the subtraction negative + before there is anything to clamp it against, and a negative is not an + amount to read or to pack. + """ + floored = nonnegative(length) + def callback(pkt: 'dict[str, Any]') -> 'int': - nominal = length(pkt) + nominal = floored(pkt) remaining = pkt.get('__length__') if not isinstance(remaining, int) or remaining < 0 or nominal <= remaining: @@ -317,15 +409,26 @@ def bounded_area(length: 'Callable[[dict[str, Any]], int]') -> 'Callable[[dict[s Note: The five non-packet blocks' option areas -- Section Header, Interface Description, Name Resolution, Interface Statistics and Decryption - Secrets -- are deliberately left unclamped here, since each computes its - span with a different offset and the equality above has to be - re-established per block rather than assumed. The general fix is to stop - a declared length reaching a read at all, which is `#678 - `__. + Secrets -- are deliberately left unclamped *against the block* here, + since each computes its span with a different offset and the equality + above has to be re-established per block rather than assumed. They do go + through :func:`nonnegative`, which is the part of `#678 + `__ that stops a + declared length reaching a read at all; the per-block equality is still + open. + + The nominal span goes through :func:`nonnegative` first here too. That + closes a hole in this function's own arithmetic: a ``captured_len`` past + the end of the block makes the span negative, and ``nominal <= available`` + below is then *true*, so the negative was returned unclamped and reached + a :mod:`struct` template as ``f'{-N}s'``. Measured on 200 Enhanced Packet + Blocks declaring ``captured_len`` ``0xFFFFFF`` in 8,048 octets. """ + floored = nonnegative(length) + def callback(pkt: 'dict[str, Any]') -> 'int': - nominal = length(pkt) + nominal = floored(pkt) remaining = pkt.get('__length__') if not isinstance(remaining, int): @@ -359,10 +462,27 @@ def pcapng_block_selector(packet: 'dict[str, Any]') -> 'Field': * :class:`pcapkit.const.pcapng.block_type.BlockType` * :class:`pcapkit.protocols.schema.misc.pcapng.BlockType` + Note: + ``__length__`` is what is left of the *stream*, not what the block + declares, and it is decremented by four for :attr:`PCAPNG.type` whether + or not those four octets were there to read -- + :meth:`FieldBase.unpack ` + zero-pads a short read rather than refusing it. A tail of one, two or + three octets therefore arrived here negative and + :meth:`io.RawIOBase.read` raised a bare ``ValueError``, which is `#678 + `__. The floor is a + backstop: :meth:`PCAPNG._check_block_floor + ` reports that + tail as end-of-stream before it gets here, and on the packing path + :meth:`Schema.pack ` seeds + ``__length__`` as ``-1`` for "unknown", which + :meth:`SchemaField.pack ` + does not read at all. + """ block_type = packet['type'] # type: Enum_BlockType schema = BlockType.registry[block_type] - return SchemaField(length=packet['__length__'], schema=schema) + return SchemaField(length=max(packet['__length__'], 0), schema=schema) def dsb_secrets_selector(packet: 'dict[str, Any]') -> 'Field': @@ -506,7 +626,7 @@ class UnknownBlock(BlockType): #: Block total length. length: 'int' = UInt32Field(callback=byteorder_callback) #: Block body (including padding). - body: 'bytes' = BytesField(length=lambda pkt: pkt['length'] - 12) + body: 'bytes' = BytesField(length=nonnegative(lambda pkt: pkt['length'] - 12)) #: Block total length. length2: 'int' = UInt32Field(callback=byteorder_callback) @@ -594,18 +714,60 @@ def register(code: 'Enum_OptionType', cls: 'Type[Option]', ns: 'Optional[str]' = ns: Namespace of option type enumeration. If not given, the value will be inferred from the option type code. + A registration that displaces another schema for the same code is + reported as a :exc:`~pcapkit.utilities.warnings.RegistryWarning`, as + every other registry in the package does -- the lookup that follows + cannot tell a deliberate replacement from an accidental one, so an + unreported overwrite is a parser silently swapped out for another. See + `#681 `__ for the same + guard on ``register_protocol``. + + Presence alone is the test here, rather than #681's presence *and a + different class*: that one keys on a name **derived** from the class, so + the wrapper registrars reach it twice with the same class on a supported + path and warning there would be noise. This keys on a caller-supplied + ``code``, and :meth:`__init_subclass__` passes each code exactly once per + subclass, so a second arrival is a second deliberate call -- which is what + the seven sibling :meth:`register` methods on + :class:`~pcapkit.protocols.protocol.ProtocolBase` and friends already + assume. + + Note: + ``ns='opt'`` fans one registration out across every namespace, so the + collision is reported once for the registration and names the + namespaces it displaced something in, rather than once per namespace. + + A namespace created by this call starts as a copy of ``opt``'s + defaults, so nothing in it is a prior registration and it is exempt: + registering an ``opt``-namespace code into a brand-new namespace is + exactly what that copy is for. + + Membership is tested with ``in``, never by subscripting. The + per-namespace registries are :class:`collections.defaultdict`\\ s -- + only the outer one is the miss-safe + :class:`~pcapkit.protocols.schema.schema._EnumRegistry` -- so reading + ``Option.registry[key][code]`` to see whether it is there would + *insert* :class:`UnknownOption` for a code nobody registered. + """ if ns is None: ns = code.name.split('_')[0] - if ns == 'opt': - for key in Option.registry: - Option.registry[key][code] = cls - elif ns in Option.registry: - Option.registry[ns][code] = cls - else: + fresh = ns != 'opt' and ns not in Option.registry + if fresh: Option.registry[ns] = Option.registry['opt'].copy() - Option.registry[ns][code] = cls + + targets = list(Option.registry) if ns == 'opt' else [ns] + + if not fresh: + clash = [key for key in targets if code in Option.registry[key]] + if clash: + warn(f'PCAP-NG: [Option {code}] option already registered in ' + f'namespace(s) {", ".join(repr(key) for key in clash)}, ' + f'overwriting with {cls!r}', RegistryWarning, stacklevel=stacklevel()) + + for key in targets: + Option.registry[key][code] = cls if TYPE_CHECKING: #: Option type. @@ -695,14 +857,15 @@ class SectionHeaderBlock(BlockType, code=Enum_BlockType.Section_Header_Block): section_length: 'int' = Int64Field(callback=shb_byteorder_callback, default=0xFFFF_FFFF_FFFF_FFFF) #: Options. options: 'list[Option]' = OptionField( - length=lambda pkt: pkt['length'] - 28, + length=nonnegative(lambda pkt: pkt['length'] - 28), base_schema=_OPT_Option, type_name='type', registry=Option.registry['opt'], eool=Enum_OptionType.opt_endofopt, ) #: Padding, sized from the ``__option_padding__`` key that OptionField generates. - padding: 'bytes' = PaddingField(length=lambda pkt: pkt.get('__option_padding__', 0)) + padding: 'bytes' = PaddingField( + length=nonnegative(lambda pkt: pkt.get('__option_padding__', 0))) #: Block total length. length2: 'int' = UInt32Field(callback=shb_byteorder_callback) @@ -1018,14 +1181,15 @@ class InterfaceDescriptionBlock(BlockType, code=Enum_BlockType.Interface_Descrip snaplen: 'int' = UInt32Field(default=0, callback=byteorder_callback) #: Options. options: 'list[Option]' = OptionField( - length=lambda pkt: pkt['length'] - 20, + length=nonnegative(lambda pkt: pkt['length'] - 20), base_schema=_IF_Option, type_name='type', registry=Option.registry['if'], eool=Enum_OptionType.opt_endofopt, ) #: Padding, sized from the ``__option_padding__`` key that OptionField generates. - padding: 'bytes' = PaddingField(length=lambda pkt: pkt.get('__option_padding__', 0)) + padding: 'bytes' = PaddingField( + length=nonnegative(lambda pkt: pkt.get('__option_padding__', 0))) #: Block total length. length2: 'int' = UInt32Field(callback=byteorder_callback) @@ -1173,7 +1337,8 @@ class EnhancedPacketBlock(BlockType, code=Enum_BlockType.Enhanced_Packet_Block): eool=Enum_OptionType.opt_endofopt, ) #: Padding, sized from the ``__option_padding__`` key that OptionField generates. - padding_opts: 'bytes' = PaddingField(length=lambda pkt: pkt.get('__option_padding__', 0)) + padding_opts: 'bytes' = PaddingField( + length=nonnegative(lambda pkt: pkt.get('__option_padding__', 0))) #: Block total length. length2: 'int' = UInt32Field(callback=byteorder_callback) @@ -1354,7 +1519,7 @@ class NameResolutionBlock(BlockType, code=Enum_BlockType.Name_Resolution_Block): length: 'int' = UInt32Field(callback=byteorder_callback) #: Name resolution records. records: 'list[NameResolutionRecord]' = OptionField( - length=lambda pkt: pkt['length'] - 12, + length=nonnegative(lambda pkt: pkt['length'] - 12), base_schema=NameResolutionRecord, type_name='type', registry=NameResolutionRecord.registry, @@ -1362,14 +1527,15 @@ class NameResolutionBlock(BlockType, code=Enum_BlockType.Name_Resolution_Block): ) #: Options. options: 'list[Option]' = OptionField( - length=lambda pkt: pkt.get('__option_padding__', 0), # key from OptionField + length=nonnegative(lambda pkt: pkt.get('__option_padding__', 0)), # key from OptionField base_schema=_NS_Option, type_name='type', registry=Option.registry['ns'], eool=Enum_OptionType.opt_endofopt, ) #: Padding, sized from the ``__option_padding__`` key that OptionField generates. - padding: 'bytes' = PaddingField(length=lambda pkt: pkt.get('__option_padding__', 0)) + padding: 'bytes' = PaddingField( + length=nonnegative(lambda pkt: pkt.get('__option_padding__', 0))) #: Block total length. length2: 'int' = UInt32Field(callback=byteorder_callback) @@ -1513,14 +1679,15 @@ class InterfaceStatisticsBlock(BlockType, code=Enum_BlockType.Interface_Statisti timestamp_low: 'int' = UInt32Field(callback=byteorder_callback) #: Options. options: 'list[Option]' = OptionField( - length=lambda pkt: pkt['length'] - 24, + length=nonnegative(lambda pkt: pkt['length'] - 24), base_schema=_ISB_Option, type_name='type', registry=Option.registry['isb'], eool=Enum_OptionType.opt_endofopt, ) #: Padding, sized from the ``__option_padding__`` key that OptionField generates. - padding: 'bytes' = PaddingField(length=lambda pkt: pkt.get('__option_padding__', 0)) + padding: 'bytes' = PaddingField( + length=nonnegative(lambda pkt: pkt.get('__option_padding__', 0))) #: Block total length. length2: 'int' = UInt32Field(callback=byteorder_callback) @@ -1537,7 +1704,7 @@ class SystemdJournalExportBlock(BlockType, code=Enum_BlockType.systemd_Journal_E #: Block total length. length: 'int' = UInt32Field(callback=byteorder_callback) #: Journal entry. - entry: 'bytes' = BytesField(length=lambda pkt: pkt['length'] - 12) + entry: 'bytes' = BytesField(length=nonnegative(lambda pkt: pkt['length'] - 12)) #: Block total length. length2: 'int' = UInt32Field(callback=byteorder_callback) @@ -1550,6 +1717,47 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Self': Returns: Revised schema. + Note: + Two ways the entry data runs out mid-field are reported rather than + raised, for the reason :func:`nonnegative` gives: a bare + :exc:`struct.error` is neither one of + :mod:`pcapkit.utilities.exceptions` nor an :exc:`EOFError`, so it + aborted the whole extraction rather than this one entry. See `#678 + `__. + + A line of nothing but NUL octets is the block's own 32-bit padding + and ends the entry. ``bytes.strip()`` takes only ASCII whitespace, + so those octets survived it and were read as the *name* of a binary + field -- which made every journal entry whose length is not a + multiple of four raise, valid or not, since the padding that follows + it has no 64-bit length prefix behind it to unpack. Measured on a + 14-octet ``MESSAGE=hello\\n`` entry, which is as ordinary as this + block gets. + + A name line whose 64-bit length prefix is itself cut short ends the + entry too. There is nothing to read past the end of the entry, so + stopping at it is what keeps the truncated block parsing. + + A binary field's length is the widest declared length in the format, + and nothing bounded it against the entry holding it: at ``2**63`` and + above :meth:`io.BytesIO.read` refuses it outright with a bare + :exc:`OverflowError` (``cannot fit 'int' into an index-sized + integer``), and below that it silently returned whatever was there -- + so the same malformed prefix was either fatal or invisible depending + only on its magnitude. It is clamped to the octets the entry has left + and reported, which is :func:`nonnegative`'s rule at the other end of + the same range. + + Field names, keys and values are decoded with ``errors='replace'`` + rather than strictly. A non-UTF-8 octet in any of the three raised a + bare :exc:`UnicodeDecodeError` -- a :exc:`ValueError`, so foreign on + both counts, and fatal to the whole extraction over one bad octet in + one field. ``'replace'`` is the option this module's own + :class:`~pcapkit.corekit.fields.strings.StringField` already takes for + the same problem, and a value that is not text is a value the writer + should have emitted as a *binary* field, so the entry is malformed + however it is read. + """ self = cast('Self', super().post_process(packet)) @@ -1560,22 +1768,59 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Self': entry_data = io.BytesIO(entry_buffer) while True: line = entry_data.readline().strip() - if not line: + if not line or not line.strip(b'\x00'): break line_split = line.split(b'=', maxsplit=1) if len(line_split) == 2: key, value = line_split - entry.add(key.decode('utf-8'), value.decode('utf-8')) + entry.add(self._decode_text(key), self._decode_text(value)) else: - length = struct.unpack(' available: + warn(f'PCAP-NG: [systemd Journal Export] binary field {line!r} ' + f'declares {length} octet(s) with {available} left in its ' + f'entry; reading {available}', SchemaWarning, + stacklevel=stacklevel()) + length = available + + entry.add(self._decode_text(line), entry_data.read(length)) entry_data.read() # Skip trailing newline. data.append(entry) self.data = data return self + @staticmethod + def _decode_text(octets: 'bytes') -> 'str': + """Decode a journal field name, key or value, reporting what did not decode. + + Args: + octets: Field name, key or value, as it came off the wire. + + Returns: + The decoded text, with any octet that is not UTF-8 replaced. + + See :meth:`post_process` for why this replaces rather than raising. + + """ + try: + return octets.decode('utf-8') + except UnicodeDecodeError as error: + warn(f'PCAP-NG: [systemd Journal Export] {octets!r} is not UTF-8 ' + f'({error.reason} at position {error.start}); replacing what did not ' + f'decode', SchemaWarning, stacklevel=stacklevel()) + return octets.decode('utf-8', errors='replace') + if TYPE_CHECKING: #: Journal entry (decoded). data: 'list[OrderedMultiDict[str, str | bytes]]' @@ -1742,14 +1987,16 @@ class DecryptionSecretsBlock(BlockType, code=Enum_BlockType.Decryption_Secrets_B options: 'list[Option]' = OptionField( # NOTE: see EnhancedPacketBlock.options on why the padding is recomputed # here instead of being read back from ``padding_data``. - length=lambda pkt: pkt['length'] - 20 - pkt['secrets_length'] - (4 - pkt['secrets_length'] % 4) % 4, + length=nonnegative(lambda pkt: pkt['length'] - 20 - pkt['secrets_length'] + - (4 - pkt['secrets_length'] % 4) % 4), base_schema=_DSB_Option, type_name='type', registry=Option.registry['dsb'], eool=Enum_OptionType.opt_endofopt, ) #: Padding, sized from the ``__option_padding__`` key that OptionField generates. - padding_opts: 'bytes' = PaddingField(length=lambda pkt: pkt.get('__option_padding__', 0)) + padding_opts: 'bytes' = PaddingField( + length=nonnegative(lambda pkt: pkt.get('__option_padding__', 0))) #: Block total length. length2: 'int' = UInt32Field(callback=byteorder_callback) @@ -1778,7 +2025,7 @@ class CustomBlock(BlockType, code=[Enum_BlockType.Custom_Block_that_rewriters_ca #: Private enterprise number. pen: 'int' = UInt32Field(callback=byteorder_callback) #: Custom data (incl. padding and options). - data: 'bytes' = BytesField(length=lambda pkt: pkt['length'] - 16) + data: 'bytes' = BytesField(length=nonnegative(lambda pkt: pkt['length'] - 16)) #: Block total length. length2: 'int' = UInt32Field(callback=byteorder_callback) @@ -1871,7 +2118,8 @@ class PacketBlock(BlockType, code=Enum_BlockType.Packet_Block): eool=Enum_OptionType.opt_endofopt, ) #: Padding, sized from the ``__option_padding__`` key that OptionField generates. - padding_opts: 'bytes' = PaddingField(length=lambda pkt: pkt.get('__option_padding__', 0)) + padding_opts: 'bytes' = PaddingField( + length=nonnegative(lambda pkt: pkt.get('__option_padding__', 0))) #: Block total length. length2: 'int' = UInt32Field(callback=byteorder_callback) diff --git a/tests/protocols/misc/test_pcapng_unit.py b/tests/protocols/misc/test_pcapng_unit.py index 644ce15bd..6de7128fe 100644 --- a/tests/protocols/misc/test_pcapng_unit.py +++ b/tests/protocols/misc/test_pcapng_unit.py @@ -7,15 +7,20 @@ import decimal import io from ipaddress import ip_address, ip_interface +import json import os import struct +import subprocess # nosec: B404 import sys +import tempfile +import textwrap import time import types import unittest +import warnings from unittest import mock -from tests._support import purge_modules, sample_path +from tests._support import ROOT, purge_modules, sample_path RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper') HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS) @@ -3614,5 +3619,879 @@ def test_the_area_bound_does_not_touch_a_well_formed_block(self) -> None: self.assertEqual(schema.captured_len, captured) +class NamedBuffer(io.BufferedReader): + """An in-memory capture with the two attributes :class:`Extractor` wants. + + :class:`~pcapkit.foundation.extraction.Extractor` reads ``fin.name`` to + derive the output name and ``fin.peek`` to sniff the file magic, neither of + which a bare :class:`io.BytesIO` has. Wrapping one keeps a 1,509-level sweep + off the filesystem: the same sweep through + :class:`tempfile.NamedTemporaryFile` costs twice the wall clock and leaves + 1,509 files behind if the process dies. + + """ + + def __init__(self, data: bytes, name: str = 'truncated.pcapng') -> None: + super().__init__(io.BytesIO(data)) + self._name = name + + @property + def name(self) -> 'str': # type: ignore[override] + return self._name + + +class Unseekable(io.RawIOBase): + """A read-only stream that reports itself unseekable.""" + + def __init__(self, data: bytes) -> None: + super().__init__() + self._data = io.BytesIO(data) + + def readable(self) -> 'bool': + return True + + def seekable(self) -> 'bool': + return False + + def readinto(self, buffer) -> 'int': # type: ignore[no-untyped-def] + return self._data.readinto(buffer) + + +@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') +class PCAPNGTruncatedFileTests(unittest.TestCase): + """A capture cut short must still report the frames before the cut. + + Block Total Length is cross-checked against its own trailing copy and never + against the file, so a last block running past the real end left the reader + seeked *past* that end -- which is legal and silent. Every block read after + it then measured a negative remainder, and the negative reached + :meth:`io.RawIOBase.read` through ``pcapng_block_selector`` as a bare + ``ValueError`` that no handler in the frame loop catches. The whole + extraction was lost, not the one truncated block, which inverts the #431 + accommodation exactly. Measured on ``dhcp.pcapng`` before the fix: of its + 1,509 octet boundaries, **6** parsed and 1,489 raised something from outside + :mod:`pcapkit.utilities.exceptions` -- 1,479 ``ValueError`` and 10 + ``struct.error``. C.f. #678, #431, #571, and #676 for the option-area bound + this was found beside. + + """ + + def setUp(self) -> None: + purge_modules(['pcapkit']) + + with open(sample_path('dhcp.pcapng'), 'rb') as stream: + self.whole = stream.read() + + def _extract(self, data: bytes): + """Extract ``data`` in memory, returning the extractor and its warnings.""" + from pcapkit.foundation.extraction import Extractor + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + extractor = Extractor(NamedBuffer(data), nofile=True, store=True) + return extractor, caught + + def _sweep(self) -> 'tuple[dict[int, int], dict[int, BaseException]]': + """Extract every truncation of the sample, one octet at a time. + + Returns the frame count for each level that parsed and the exception for + each that did not. Sweeping every boundary rather than picking one is the + point: the levels that behave differently are not the ones anybody would + have chosen by hand -- 372 and 373 are where ``struct.error`` lived, and + 376 is where the cut happens to land on a block boundary. + + """ + frames = {} # type: dict[int, int] + failures = {} # type: dict[int, BaseException] + + for cut in range(len(self.whole) + 1): + data = self.whole[:len(self.whole) - cut] + try: + extractor, _ = self._extract(data) + except BaseException as error: # noqa: BLE001 -- classifying, not handling + failures[cut] = error + else: + frames[cut] = len(extractor.frame) + + self.assertEqual(len(frames) + len(failures), len(self.whole) + 1) + return frames, failures + + def test_no_truncation_level_raises_from_outside_the_library(self) -> None: + """The whole of #678, asserted over every boundary rather than one. + + A caller cannot tell a bare ``ValueError`` or ``struct.error`` from a bug + in its own code, and neither is an :exc:`EOFError`, so neither is caught + where the frame loop catches the end of a file. Every failure that is + left has to come from :mod:`pcapkit.utilities.exceptions`. + + """ + from pcapkit.utilities.exceptions import BaseError + + _, failures = self._sweep() + + foreign = {cut: error for cut, error in failures.items() + if not isinstance(error, BaseError)} + self.assertEqual( + foreign, {}, + f'{len(foreign)} truncation level(s) raised from outside ' + f'pcapkit.utilities.exceptions: ' + f'{sorted((cut, type(err).__name__) for cut, err in foreign.items())[:8]}' + ) + + def test_almost_every_truncation_level_parses(self) -> None: + """The #431 bar: a truncated capture parses, it does not fail. + + Absence of a foreign exception would be satisfied by turning all 1,509 + levels into a tidy in-library refusal, which is the failure this guards + against. Before the fix six levels parsed; the rest is what the fix is + for. + + """ + frames, failures = self._sweep() + + self.assertGreater(len(frames), 0.98 * (len(self.whole) + 1)) + self.assertLess(len(failures), 16) + + def test_every_level_that_still_fails_fails_for_a_reason_of_its_own(self) -> None: + """The handful left are all cuts that reach the section header itself. + + None of them costs a frame that was in the file: a cut that deep has + already removed every packet block. A file under twelve octets cannot + hold a block at all and is reported as end-of-stream; under four it + cannot even be identified as PCAP-NG. + + """ + from pcapkit.utilities.exceptions import (FormatError, ProtocolError, + StreamEOFError) + + frames, failures = self._sweep() + + for cut, error in sorted(failures.items()): + with self.subTest(cut=cut, error=type(error).__name__): + self.assertIsInstance(error, (StreamEOFError, ProtocolError, FormatError)) + self.assertGreater(cut, len(self.whole) - 64) + + # every level that failed is deeper than every level that still had a + # frame to report, so no failure cost a frame that was in the file + deepest_with_a_frame = max(cut for cut, count in frames.items() if count) + self.assertLess(deepest_with_a_frame, min(failures)) + + def test_the_frame_count_never_rises_as_the_cut_deepens(self) -> None: + """Removing octets may lose frames; it may not invent them. + + The structural property a zero-padded short read could break: padding a + shortfall out of nothing is how a block gets fabricated, and a fabricated + block would show up here as a frame count going *up* while the file got + smaller. + + """ + frames, _ = self._sweep() + + levels = sorted(frames) + for deeper, shallower in zip(levels[1:], levels): + with self.subTest(cut=deeper): + self.assertLessEqual(frames[deeper], frames[shallower]) + + self.assertEqual(frames[0], 4) + self.assertEqual(frames[max(levels)], 0) + + def test_a_truncated_last_block_keeps_the_frames_before_it(self) -> None: + """The levels the issue reports, with the frame counts they should give. + + Four octets off the end truncates the fourth Enhanced Packet Block, whose + declared length then overruns the file; 376 removes it entirely, landing + the cut on a block boundary. + + """ + shallow, _ = self._extract(self.whole[:-4]) + boundary, _ = self._extract(self.whole[:-376]) + + self.assertEqual(len(shallow.frame), 4) + self.assertEqual(len(boundary.frame), 3) + + def test_a_block_overrunning_the_file_is_reported_and_leaves_the_reader_at_the_end(self) -> None: + """The root cause, on its own: the seek that used to go past the end. + + ``_read_fileng`` stops at the end of the file, so the octets it returned + are the authority on where a truncated block really finishes -- and + landing the reader there is what lets the *next* read measure a remainder + of zero and report the quiet end-of-stream the frame loop already + handles, instead of a negative one. + + """ + from pcapkit.utilities.warnings import ProtocolWarning + + _, caught = self._extract(self.whole[:-4]) + + overruns = [str(entry.message) for entry in caught + if entry.category is ProtocolWarning + and 'octet(s) left in the file' in str(entry.message)] + self.assertEqual(len(overruns), 1) + self.assertIn('block length 376 exceeds the 372 octet(s) left', overruns[0]) + + def test_a_tail_too_short_for_a_block_is_reported_as_end_of_stream(self) -> None: + """Twelve octets is the smallest block there is, so eleven is not one. + + Reported as :exc:`~pcapkit.utilities.exceptions.StreamEOFError` rather + than clamped, because at the end of the file no block is being read: the + clamp-and-warn of #676 keeps a truncated *block* parsing, where inventing + a whole block out of zero padding would only fabricate a frame. It is the + same signal ``prepare`` already raises for a remainder of exactly zero -- + one, two and three octets merely fell through it. + + """ + from pcapkit.protocols.misc.pcapng import PCAPNG + from pcapkit.utilities.exceptions import StreamEOFError + + for size in range(12): + with self.subTest(octets=size): + with self.assertRaises(StreamEOFError) as caught: + PCAPNG(bytes(size), num=1, sct=1, ctx=None) + + self.assertIn(f'{size} octet(s) left', str(caught.exception)) + # an EOFError, which is what the frame loop catches + self.assertIsInstance(caught.exception, EOFError) + + def test_the_smallest_possible_block_is_not_mistaken_for_the_end(self) -> None: + """Twelve octets is a block, so the floor may not reject it. + + An off-by-one here would stop every well-formed + :class:`~pcapkit.protocols.schema.misc.pcapng.UnknownBlock` of minimum + size, and ``__length_hint__`` reports the same twelve. + + """ + from pcapkit.protocols.misc.pcapng import PCAPNG + + raw = struct.pack(' None: + """Measuring the remainder needs a seek, and nothing here guards that. + + It does not have to: + :meth:`~pcapkit.protocols.misc.pcapng.PCAPNG.__post_init__` has already + called :meth:`~io.IOBase.tell` on the stream to record ``_seek_set`` + before ``unpack`` runs, so an unseekable stream never reaches the floor. + Pinned rather than assumed, because a ``seekable()`` guard on the floor + would have been unreachable code -- and because the behaviour it would + have been protecting is unchanged by this fix. + + """ + from pcapkit.protocols.misc.pcapng import PCAPNG + + stream = Unseekable(bytes(3)) + self.assertFalse(stream.seekable()) + + with self.assertRaises(io.UnsupportedOperation): + PCAPNG(stream, num=1, sct=1, ctx=None) # type: ignore[arg-type] + + def test_the_block_floor_does_not_fire_while_constructing(self) -> None: + """Construction builds its own buffer, so the parsing floor is not its business. + + ``PCAPNG(file=None, ...)`` packs the block and reads back what it packed. + A Section Header Block is 28 octets, comfortably over the floor, but the + floor is skipped there on principle rather than on size: a block short + enough to trip it is already reported by + :meth:`~pcapkit.protocols.misc.pcapng.PCAPNG.read`'s own Block Total + Length check, which names the length and so says more than an + end-of-stream would. + + """ + from pcapkit.const.pcapng.block_type import BlockType + from pcapkit.protocols.misc.pcapng import PCAPNG + + block = PCAPNG(num=0, sct=1, ctx=None, + type=BlockType.Section_Header_Block, block={}) + + self.assertEqual(block.info.length, 28) + self.assertEqual(len(bytes(block)), 28) + + +@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') +class PCAPNGNegativeLengthTests(unittest.TestCase): + """No computed field length may reach :mod:`struct` or ``read()`` negative. + + Every span in the PCAP-NG schema is a subtraction whose operands are wire + fields, and nothing made the difference non-negative. The field layer does + not either: ``_TextField.__call__`` builds its template as ``f'{length}s'`` + unconditionally, so ``-8`` becomes the format ``'-8s'`` and + :func:`struct.calcsize` raises a bare :exc:`struct.error` -- which is neither + in :mod:`pcapkit.utilities.exceptions` nor an :exc:`EOFError`, so one + malformed block cost the whole extraction. The second manifestation on #678: + an Enhanced Packet Block declaring ``captured_len`` past its own end. + + """ + + #: Every key a length callback in the module reads, set so that each + #: subtraction comes out negative: a Block Total Length of zero is under + #: every block's fixed-field floor, a ``captured_len`` of ``0xFFFFFF`` is + #: past the end of any real block, and ``-32`` is what ``OptionField`` + #: reports as ``__option_padding__`` when its options overran their area. + #: + #: ``__length__`` stays non-negative deliberately. It is the one key that is + #: a measurement rather than a difference -- ``prepare`` sets it from what is + #: left of the stream, or from the length the enclosing + #: :class:`~pcapkit.corekit.fields.misc.SchemaField` declared -- so flooring a + #: field that reads it *whole* changes nothing on the parsing path and + #: truncates on the packing one, where ``Schema.pack`` leaves it at ``-1`` for + #: "unknown". See + #: :meth:`test_a_field_sized_by_the_remaining_length_whole_is_left_alone`. + HOSTILE = { + 'length': 0, + 'captured_len': 0xFFFFFF, + 'captured_length': 0xFFFFFF, + 'secrets_length': 0xFFFFFF, + '__option_padding__': -32, + '__length__': 0, + 'packet_data': b'', + 'snaplen': 0, + } + + def setUp(self) -> None: + purge_modules(['pcapkit']) + + def _hostile_packet(self): + """``HOSTILE``, with zero for any key it does not name.""" + class Hostile(dict): + def __missing__(self, key): + return 0 + + return Hostile(self.HOSTILE) + + def test_every_length_callback_in_the_module_is_floored_at_zero(self) -> None: + """Exhaustive over the module, so a new unfloored subtraction is caught. + + Walks every schema in + :mod:`pcapkit.protocols.schema.misc.pcapng` and drives every ``length`` + callback with the hostile packet above. Measured on the parent commit: 28 + of the 74 callbacks returned a negative, from ``-1`` on an ``epb_hash`` + declaring no payload to ``-16777248`` on an Enhanced Packet Block's + option area. + + """ + from pcapkit.protocols.schema.misc import pcapng as module + from pcapkit.protocols.schema.schema import Schema + + packet = self._hostile_packet() + exercised = [] + negative = [] + + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + for name, obj in vars(module).items(): + if not (isinstance(obj, type) and issubclass(obj, Schema)): + continue + for field_name, field in getattr(obj, '__fields__', {}).items(): + callback = getattr(field, '_length_callback', None) + if callback is None: # a fixed-width field has nothing to compute + continue + label = f'{name}.{field_name}' + value = callback(packet) + exercised.append(label) + if value < 0: + negative.append((label, value)) + + self.assertEqual(negative, []) + # the sweep has to have found the callbacks at all: an import that + # renamed the module's schemas would otherwise pass by exercising none + self.assertGreaterEqual(len(exercised), 70) + + def test_a_field_sized_by_the_remaining_length_whole_is_left_alone(self) -> None: + """The three decryption-secrets payloads are exempt, and measurably so. + + ``UnknownSecrets.data``, ``TLSKeyLog.data`` and ``WireGuardKeyLog.data`` + read ``__length__`` whole rather than subtracting from it, and + :meth:`Schema.pack ` leaves + it at ``-1`` when no length is known -- so flooring them at zero packs + *nothing*. Measured: it emptied both secrets payloads, and the two + ``EXPECTED_FAILURES`` entries for them in + ``tests/protocols/test_option_roundtrip_unit.py`` then came back ``OK`` + rather than ``MISMATCH``, because an empty payload compares equal to an + empty payload. A regression that reads as a fix, which is why this is + pinned rather than left to the reader. + + """ + from pcapkit.protocols.schema.misc.pcapng import TLSKeyLog, UnknownSecrets + + for schema_cls, field_name in ((UnknownSecrets, 'data'), (TLSKeyLog, 'data')): + with self.subTest(schema=schema_cls.__name__): + callback = schema_cls.__fields__[field_name]._length_callback + self.assertEqual(callback({'__length__': -1}), -1) + self.assertEqual(callback({'__length__': 40}), 40) + + def test_the_floor_warns_rather_than_shortening_a_read_in_silence(self) -> None: + """A negative length means the file is malformed, and that is worth saying. + + The clamp is not a no-op the way :func:`bounded_area`'s is on a + well-formed block: it only ever fires on framing that contradicts itself, + so the warning cannot be routine noise. + + """ + from pcapkit.protocols.schema.misc.pcapng import nonnegative + from pcapkit.utilities.warnings import SchemaWarning + + callback = nonnegative(lambda pkt: pkt['length'] - 12) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + clamped = callback({'length': 4}) + untouched = callback({'length': 40}) + + self.assertEqual(clamped, 0) + self.assertEqual(untouched, 28) + self.assertEqual([entry.category for entry in caught], [SchemaWarning]) + self.assertIn('-8', str(caught[0].message)) + + def test_an_epb_declaring_captured_len_past_its_block_does_not_reach_struct(self) -> None: + """#678's second manifestation, at the field that raised it. + + ``length - 32 - captured_len - padding`` goes negative, and + :func:`bounded_area`'s own ``nominal <= available`` test was *true* for a + negative nominal, so it returned it unclamped. + + """ + from pcapkit.protocols.schema.misc.pcapng import EnhancedPacketBlock + + body = (struct.pack(' None: + """The other shape: a declared length under the block's fixed fields. + + Each of these blocks sizes its option area or body as the declared length + less a different offset, so each subtraction has to be floored in its own + right -- 28 octets for a Section Header Block, 20 for an Interface + Description Block, 12 for a Name Resolution Block and a systemd journal + export, 24 for Interface Statistics, 16 for a Custom Block. + + """ + from pcapkit.protocols.schema.misc.pcapng import (CustomBlock, + InterfaceDescriptionBlock, + InterfaceStatisticsBlock, + NameResolutionBlock, + SectionHeaderBlock, + SystemdJournalExportBlock, + UnknownBlock) + + bodies = { + SectionHeaderBlock: struct.pack(' bytes: + """A systemd Journal Export Block carrying ``entry``, padded to 32 bits.""" + body = entry + bytes(-len(entry) % 4) + length = 12 + len(body) + return struct.pack(' None: + """A 32-bit-unaligned journal entry used to raise a bare ``struct.error``. + + The block body is padded to a 32-bit boundary with NULs, and + ``bytes.strip()`` takes only ASCII whitespace -- so the padding survived + it and was read as the *name* of a binary field, whose 64-bit length + prefix then had nothing behind it. ``MESSAGE=hello\\n`` is 14 octets, so + two NULs follow it, and that is as ordinary as this block gets: the + failure was not confined to malformed input. + + """ + entries, caught = self._extract_journal(b'MESSAGE=hello\n') + + self.assertEqual(len(entries), 1) + self.assertEqual(entries[0]['MESSAGE'], 'hello') + # and silently: nothing about a well-formed entry is worth warning over + self.assertEqual([entry.category.__name__ for entry in caught + if entry.category.__name__ == 'SchemaWarning'], []) + + def test_a_journal_binary_field_cut_short_of_its_length_is_reported(self) -> None: + """The reviewer's case: fewer than eight octets for a 64-bit length. + + ``struct.unpack('= 8: + # the block's own padding made the prefix up to eight + self.assertEqual(schema_warnings, []) + continue + self.assertEqual(len(schema_warnings), 1) + self.assertIn('of the 8 it needs', str(schema_warnings[0].message)) + self.assertEqual(len(entries), 1) + self.assertEqual(len(entries[0]), 0) + + def test_a_well_formed_journal_binary_field_still_reads_its_value(self) -> None: + """The guard is a shortfall check, not a refusal of binary fields.""" + entries, caught = self._extract_journal( + b'MESSAGE\n' + struct.pack(' None: + """A 64-bit length was either fatal or invisible, by magnitude alone. + + At ``2**63`` and above :meth:`io.BytesIO.read` refuses the length outright + with a bare :exc:`OverflowError`; below that it silently returned whatever + happened to be there. Both are the same malformed prefix, so both get the + same answer: clamp to the octets the entry has left, and say so. + + """ + from pcapkit.utilities.warnings import SchemaWarning + + # ``b'BINARY\n' + 8 octets + b'abc\n'`` is 19 octets, so the block pads it + # with one NUL and five octets follow the length prefix + for declared in (6, 1 << 10, 1 << 30, 1 << 62, 1 << 63, 2 ** 64 - 1): + with self.subTest(declared=declared, expect='clamped'): + entries, caught = self._extract_journal( + b'BINARY\n' + struct.pack(' None: + """One bad octet in one field used to cost the whole extraction. + + :exc:`UnicodeDecodeError` is a :exc:`ValueError`, so it is neither in + :mod:`pcapkit.utilities.exceptions` nor an :exc:`EOFError`. Asserted for + all three sites that decode -- a field name, a key and a value. + + """ + from pcapkit.utilities.warnings import SchemaWarning + + cases = { + 'key': (b'ME\xffSAGE=hello\n', 'ME�SAGE', 'hello'), + 'value': (b'MESSAGE=hel\xfflo\n', 'MESSAGE', 'hel�lo'), + 'binary field name': (b'BIN\xffARY\n' + struct.pack(' None: + """#594's amplification band, on the ``captured_len`` vector. + + The issue's own measurement -- 200 Enhanced Packet Blocks declaring + ``captured_len`` ``0xFFFFFF`` over eight real octets, in 8,048 octets. Run + in a subprocess under :data:`resource.RLIMIT_AS`, because the failure mode + this area is guarded against is synthesising padding without bound: an + in-process regression would take the test host with it rather than + failing, and a cap that the parent cannot lift is the only way to tell + "parsed" from "did not run out of memory yet". + + """ + if importlib.util.find_spec('resource') is None: # pragma: no cover + self.skipTest('the resource module is unavailable on this platform') + + source = textwrap.dedent(''' + import io, json, resource, struct, sys, warnings + sys.path.insert(0, sys.argv[1]) + resource.setrlimit(resource.RLIMIT_AS, (512 * 1024 ** 2,) * 2) + + import pcapkit + from pcapkit.foundation.extraction import Extractor + + # the tree under test is the one this was pointed at, not an + # installed copy: asserted here rather than compared across the + # process boundary, where the two can differ only in path form + assert pcapkit.__file__.startswith(sys.argv[1]), pcapkit.__file__ + + shb = struct.pack(' None: + purge_modules(['pcapkit']) + + def _snapshot(self): + """Restore every namespace dictionary on teardown.""" + from pcapkit.protocols.schema.misc.pcapng import Option + + saved = {key: value.copy() for key, value in Option.registry.items()} + names = set(Option.registry) + + def restore() -> 'None': + for key, value in saved.items(): + Option.registry[key].clear() + Option.registry[key].update(value) + for key in set(Option.registry) - names: + del Option.registry[key] + + self.addCleanup(restore) + return Option + + def test_a_first_registration_is_silent(self) -> None: + """The guard may not fire on the path that builds the registry. + + Every option schema in the module registers itself through + ``__init_subclass__``, so a guard that warns on a code nobody registered + would warn once per option at import -- and the filter that invites is + what would hide a real collision. Measured: importing + :mod:`pcapkit` emits no + :exc:`~pcapkit.utilities.warnings.RegistryWarning`. + + """ + from pcapkit.const.pcapng.option_type import OptionType + from pcapkit.protocols.schema.misc.pcapng import UnknownOption + from pcapkit.utilities.warnings import RegistryWarning + + option = self._snapshot() + unregistered = OptionType.get(0x00FA, namespace='if') + self.assertNotIn(unregistered, option.registry['if']) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + option.register(unregistered, UnknownOption, ns='if') + + self.assertEqual([entry for entry in caught + if entry.category is RegistryWarning], []) + self.assertIs(option.registry['if'][unregistered], UnknownOption) + + def test_re_registering_an_option_code_reports_the_collision(self) -> None: + """Presence alone is the test, as it is for the seven sibling registrars. + + #681 tested presence *and a different class* because it keys on a name + derived from the class, which the wrapper registrars reach twice with the + same class. This keys on a caller-supplied code that + ``__init_subclass__`` passes exactly once per subclass, so a second + arrival is a second deliberate call -- and reporting it is what the + ``ProtocolBase``, ``Frame``, ``Internet``, ``PCAPNG``, ``SCTP``, + ``Transport`` and ``Link`` registrars already do. + + """ + from pcapkit.const.pcapng.option_type import OptionType + from pcapkit.protocols.schema.misc.pcapng import UnknownOption + from pcapkit.utilities.warnings import RegistryWarning + + option = self._snapshot() + incumbent = option.registry['if'][OptionType.if_name] + self.assertIsNot(incumbent, UnknownOption) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + option.register(OptionType.if_name, UnknownOption) + + registry_warnings = [entry for entry in caught + if entry.category is RegistryWarning] + self.assertEqual(len(registry_warnings), 1) + self.assertIn("namespace(s) 'if'", str(registry_warnings[0].message)) + self.assertIs(option.registry['if'][OptionType.if_name], UnknownOption) + + def test_an_opt_namespace_collision_names_every_namespace_it_displaced(self) -> None: + """``ns='opt'`` is one registration, so it gets one warning. + + The ``opt`` namespace fans a single registration out across every + namespace there is, and a warning per namespace would report one mistake + seven times. Naming them in one message says the same thing and stays + greppable. + + """ + from pcapkit.const.pcapng.option_type import OptionType + from pcapkit.protocols.schema.misc.pcapng import UnknownOption + from pcapkit.utilities.warnings import RegistryWarning + + option = self._snapshot() + namespaces = list(option.registry) + self.assertGreater(len(namespaces), 1) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + option.register(OptionType.opt_comment, UnknownOption, ns='opt') + + registry_warnings = [entry for entry in caught + if entry.category is RegistryWarning] + self.assertEqual(len(registry_warnings), 1) + for namespace in namespaces: + self.assertIn(repr(namespace), str(registry_warnings[0].message)) + for namespace in namespaces: + self.assertIs(option.registry[namespace][OptionType.opt_comment], + UnknownOption) + + def test_a_namespace_created_by_the_registration_is_not_a_collision(self) -> None: + """A fresh namespace starts as a copy of ``opt``'s defaults. + + Those defaults are not prior registrations, and putting an ``opt`` code + into a new namespace is exactly what the copy is for -- so warning there + would report the supported path as a mistake. + + """ + from pcapkit.const.pcapng.option_type import OptionType + from pcapkit.protocols.schema.misc.pcapng import UnknownOption + from pcapkit.utilities.warnings import RegistryWarning + + option = self._snapshot() + self.assertNotIn('brand_new', option.registry) + self.assertIn(OptionType.opt_comment, option.registry['opt']) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + option.register(OptionType.opt_comment, UnknownOption, ns='brand_new') + + self.assertEqual([entry for entry in caught + if entry.category is RegistryWarning], []) + self.assertIs(option.registry['brand_new'][OptionType.opt_comment], + UnknownOption) + + def test_the_collision_check_does_not_insert_a_default(self) -> None: + """Membership with ``in``, never by subscripting. + + Only the outer registry is the miss-safe ``_EnumRegistry``; the + per-namespace ones are plain :class:`collections.defaultdict`\\ s, so + reading ``Option.registry[ns][code]`` to see whether a code is there + *inserts* ``UnknownOption`` for it -- the schema-layer form of the + #421/#425/#428 defect. + + """ + from pcapkit.const.pcapng.option_type import OptionType + from pcapkit.protocols.schema.misc.pcapng import UnknownOption + + option = self._snapshot() + before = {key: len(value) for key, value in option.registry.items()} + + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + option.register(OptionType.if_name, UnknownOption) + + after = {key: len(value) for key, value in option.registry.items()} + self.assertEqual(after, before) + + if __name__ == '__main__': unittest.main()