fix(link,internet): reject a bool at the four remaining maker call sites (#540)

CHANGELOG.md

@@ -28,6 +28,7 @@ The largest release since 1.0, and the first recorded here as it happened rather
 - **Fixed** -- a truncated or under-declared area no longer parses "successfully", and no longer wedges the process. The option and list loops could spin forever with no exception on a truncated area, reachable from untrusted input through HOPOPT, IPv6-Opts, MH, HIP and SCTP; each iteration must now advance the stream by at least one octet, and the error names the option, the offset and the octets remaining (#431, #432). Separately, wire-derived lengths in `ipv6_opts`, CALIPSO, MPL, REG_INFO and four HIP list callbacks underflowed below zero, which `ListField`'s own `while length > 0` then turned into a silent empty list; they are floored and raise instead (#449, #456, #460, #463).
 - **Fixed** -- field widths and units, each measured against the specification rather than inferred: HIP's `TRANSPORT_FORMAT_LIST`, `NAT_TRAVERSAL_MODE` and `ESP_TRANSFORM` list entries are two octets, not one [[RFC 7401](https://datatracker.ietf.org/doc/html/rfc7401), [RFC 5770](https://datatracker.ietf.org/doc/html/rfc5770), [RFC 7402](https://datatracker.ietf.org/doc/html/rfc7402)] (#463, #472); the MN-ID option sizes from its subtype, not from `identifier`'s Python type (#448, #464, #467); IPv6-Route's `Hdr Ext Len` is computed in 8-octet units on both sides [[RFC 8200](https://datatracker.ietf.org/doc/html/rfc8200)] (#487, #489); and the Fast Binding Update and Acknowledgment Lifetimes are plain seconds [[RFC 5568](https://datatracker.ietf.org/doc/html/rfc5568)], not [RFC 6275](https://datatracker.ietf.org/doc/html/rfc6275)'s four-second units (#502).
 - **Fixed** -- stdlib exceptions leaking out where the library's own were promised: a malformed IP field value raised a bare `ValueError` instead of `FieldValueError` (#465); a `bool` address was silently packed as `0.0.0.1` or `0.0.0.0`, `bool` being an `int` subclass (#491, #500); and `@prepare` discarded extra arguments silently and treated a *declared* zero length as end of stream, which is now distinguished from a genuinely exhausted one and raises `StreamEOFError` (#454, #458).
+- **Fixed** -- the last four call sites where a `_make_*` method converts an address itself, ahead of the schema, so the same `bool`-as-`int` mistake reached them too, unguarded by the schema-layer fix above: `ARP._make_proto_resolve`, `IPv6_Route._make_data_type_rpl` (which also derives its RPL compression lengths from the laundered value, corrupting the address list alongside the exception it should have raised), the `dst` parameter of `IPv6_Route.make`, and the latent `OSPF._make_id_numbers`, which nothing calls yet but would have inherited the defect regardless. All four now go through `parse_ip_address`, the helper #539 and #552 route their own sites through, completing the programme #481 and #508 started. Pinning `version=6` on the two `IPv6_Route` sites is a second, smaller behaviour change beyond bool rejection: both previously converted a plain integer through the bare, family-inferring `ipaddress.ip_address`, which let `ip=[258]` pack a 4-octet IPv4 address (`0.0.1.2`) inside an IPv6-only header; it now packs the 16-octet IPv6 form (`::102`) instead, which is what an IPv6-only header should hold regardless of what an integer argument happens to fit as an IPv4 address (#540).
 - **Fixed** -- the engine adapters, which were quietly wrong rather than loud. The `dpkt` toolkit split TCP and IPv4 headers at their fixed struct size instead of their real length, so option octets overwrote payload in the sequence-indexed reassembly buffer; it also read an `ipv6_frag.nh` that `dpkt` does not have, and passed fragment offsets unscaled (#351, #370, #385, #395). `scapy` never loaded its layer registry, so that engine did not dissect at all (#409), and its IPv4 fragment offset reached reassembly unscaled (#483, #484). The four IPv6 adapters disagreed about whether the 8-octet Fragment header belongs to `ihl`, `header` and `tl`; per [RFC 8200](https://datatracker.ietf.org/doc/html/rfc8200) section 4.5 it belongs to none of them, and all four now agree (#415, #424).
 - **Fixed** -- PCAP and PCAP-NG output and parsing: `bytes(frame)` returned the *next* frame's octets, `files=True` wrote names like `Frame 1..json`, and a PCAP-NG `timestamp_epoch` was shifted by the reading host's timezone (#403); seven further parser defects (#341--#347, #371) and, on the write path, four more block-parsing ones (#388); `BitField` packed every named bit as set (#359, #374); the extension-header walk failed to advance past the last IPv6 extension header (#348, #373); and IPv6 fragment offsets went unscaled, with reassembly keyed on the flow label -- optional, and routinely zero, so distinct datagrams collapsed together -- rather than on the fragment identification (#389).
 - **Fixed** -- TCP reassembly mixed absolute sequence numbers with buffer-relative slicing, so on any capture carrying a SYN with a realistic initial sequence number incomplete datagrams were dropped silently, and the `completed=False` branch of the public API was unreachable (#349, #376).

docs/source/changelog/1.5.0.rst

@@ -242,6 +242,23 @@ pull requests between #326 and #509.
   and ``@prepare`` discarded extra arguments silently and treated a *declared*
   zero length as end of stream, which is now distinguished from a genuinely
   exhausted one and raises ``StreamEOFError`` (#454, #458).
+* **Fixed** -- the last four call sites where a ``_make_*`` method converts an
+  address itself, ahead of the schema, so the same ``bool``-as-``int`` mistake
+  reached them too, unguarded by the schema-layer fix above:
+  ``ARP._make_proto_resolve``, ``IPv6_Route._make_data_type_rpl`` (which also
+  derives its RPL compression lengths from the laundered value, corrupting the
+  address list alongside the exception it should have raised), the ``dst``
+  parameter of ``IPv6_Route.make``, and the latent ``OSPF._make_id_numbers``,
+  which nothing calls yet but would have inherited the defect regardless. All
+  four now go through ``parse_ip_address``, the helper #539 and #552 route
+  their own sites through, completing the programme #481 and #508 started.
+  Pinning ``version=6`` on the two ``IPv6_Route`` sites is a second, smaller
+  behaviour change beyond bool rejection: both previously converted a plain
+  integer through the bare, family-inferring ``ipaddress.ip_address``, which
+  let ``ip=[258]`` pack a 4-octet IPv4 address (``0.0.1.2``) inside an
+  IPv6-only header; it now packs the 16-octet IPv6 form (``::102``) instead,
+  which is what an IPv6-only header should hold regardless of what an integer
+  argument happens to fit as an IPv4 address (#540).
 * **Fixed** -- the engine adapters, which were quietly wrong rather than loud.
   The ``dpkt`` toolkit split TCP and IPv4 headers at their fixed struct size
   instead of their real length, so option octets overwrote payload in the

pcapkit/protocols/internet/ipv6_route.py

@@ -23,13 +23,13 @@
 
 """
 import collections
-import ipaddress
 import math
 import os.path as os_path
 from typing import TYPE_CHECKING, cast, overload
 
 from pcapkit.const.ipv6.routing import Routing as Enum_Routing
 from pcapkit.const.reg.transtype import TransType as Enum_TransType
+from pcapkit.corekit.fields.ipaddress import parse_ip_address
 from pcapkit.protocols.data.internet.ipv6_route import RPL as Data_RPL
 from pcapkit.protocols.data.internet.ipv6_route import IPv6_Route as Data_IPv6_Route
 from pcapkit.protocols.data.internet.ipv6_route import SourceRoute as Data_SourceRoute
@@ -286,6 +286,10 @@ def make(self,
         Returns:
             Constructed packet data.
 
+        Raises:
+            FieldValueError: If ``dst`` is a :obj:`bool` (c.f.
+                :func:`~pcapkit.corekit.fields.ipaddress.parse_ip_address`).
+
         """
         next_val = cast('Enum_TransType',
                         self._make_index(next, next_default, namespace=next_namespace,
@@ -311,7 +315,13 @@ def make(self,
             else:
                 meth = name[1]
 
-            dst_val = cast('IPv6Address', ipaddress.ip_address(dst)) if dst is not None else None
+            # NOTE: Through ``parse_ip_address`` rather than
+            # ``ipaddress.ip_address`` directly, because ``bool`` is an
+            # ``int`` subclass the latter accepts without complaint. Before
+            # this, ``dst=True`` converted to ``::1`` with no exception at
+            # all (c.f. #508, #540).
+            dst_val = cast('IPv6Address', parse_ip_address(
+                dst, f'{self.alias}: invalid destination address', version=6)) if dst is not None else None
             if isinstance(data, dict):
                 data_val = meth(type_val, dst=dst_val, **data)
             else:
@@ -718,6 +728,10 @@ def _make_data_type_rpl(self, type: 'Enum_Routing', route: 'Optional[Data_RPL]'
         Returns:
             Constructed route data schema.
 
+        Raises:
+            FieldValueError: If an entry of ``ip`` is a :obj:`bool` (c.f.
+                :func:`~pcapkit.corekit.fields.ipaddress.parse_ip_address`).
+
         """
         if route is not None:
             cmpr_i = route.cmpr_i
@@ -729,28 +743,38 @@ def _make_data_type_rpl(self, type: 'Enum_Routing', route: 'Optional[Data_RPL]'
         else:
             ip = [] if ip is None else ip
 
+            # NOTE: Through ``parse_ip_address`` rather than
+            # ``ipaddress.ip_address`` directly, because ``bool`` is an
+            # ``int`` subclass the latter accepts without complaint -- and
+            # here the laundered value would not just pack wrong, it would
+            # feed ``cmpr_i``/``cmpr_e`` below, corrupting the compression
+            # metadata alongside the address list. Before this,
+            # ``ip=[True]`` packed with ``cmpr_e=0`` and an address of
+            # ``00000001`` instead of raising (c.f. #508, #540).
+            descr = f'{self.alias}: invalid RPL source address'
+
             if dst is None:
                 pad = 0
                 cmpr_i = 0
                 cmpr_e = 0
                 ip_val = [
-                    cast('IPv6Address', ipaddress.ip_address(addr)).packed for addr in ip
+                    cast('IPv6Address', parse_ip_address(addr, descr, version=6)).packed for addr in ip
                 ]
             else:
                 test_list = [dst.packed]  # type: list[bytes]
                 for item in ip[:-1]:
                     if isinstance(item, bytes):
                         test_list.append(item)
                     else:
-                        test_list.append(cast('IPv6Address', ipaddress.ip_address(item)).packed)
+                        test_list.append(cast('IPv6Address', parse_ip_address(item, descr, version=6)).packed)
                 prefix_i = os_path.commonprefix(test_list)
                 cmpr_i = len(prefix_i)
 
                 test_list = [dst.packed]
                 if isinstance(ip[-1], bytes):
                     test_list.append(ip[-1])
                 else:
-                    test_list.append(cast('IPv6Address', ipaddress.ip_address(ip[-1])).packed)
+                    test_list.append(cast('IPv6Address', parse_ip_address(ip[-1], descr, version=6)).packed)
                 prefix_e = os_path.commonprefix(test_list)
                 cmpr_e = len(prefix_e)
 
@@ -761,11 +785,11 @@ def _make_data_type_rpl(self, type: 'Enum_Routing', route: 'Optional[Data_RPL]'
                     if isinstance(item, bytes):
                         ip_val.append(item[cmpr_i:])
                     else:
-                        ip_val.append(cast('IPv6Address', ipaddress.ip_address(item)).packed[cmpr_i:])
+                        ip_val.append(cast('IPv6Address', parse_ip_address(item, descr, version=6)).packed[cmpr_i:])
                 if isinstance(ip[-1], bytes):
                     ip_val.append(ip[-1][cmpr_e:])
                 else:
-                    ip_val.append(cast('IPv6Address', ipaddress.ip_address(ip[-1])).packed[cmpr_e:])
+                    ip_val.append(cast('IPv6Address', parse_ip_address(ip[-1], descr, version=6)).packed[cmpr_e:])
 
         return Schema_RPL(
             cmpr_i=cmpr_i,

pcapkit/protocols/link/arp.py

@@ -41,11 +41,12 @@
 import re
 import sys
 import textwrap
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, cast
 
 from pcapkit.const.arp.hardware import Hardware as Enum_Hardware
 from pcapkit.const.arp.operation import Operation as Enum_Operation
 from pcapkit.const.reg.ethertype import EtherType as Enum_EtherType
+from pcapkit.corekit.fields.ipaddress import parse_ip_address
 from pcapkit.protocols.data.link.arp import ARP as Data_ARP
 from pcapkit.protocols.data.link.arp import Address as Data_Address
 from pcapkit.protocols.data.link.arp import Type as Data_Type
@@ -395,11 +396,35 @@ def _make_proto_resolve(self, addr: 'IPv4Address | IPv6Address | str | bytes', p
             object; otherwise, returns a raw :data:`str` representing the
             protocol address.
 
+        Raises:
+            FieldValueError: If ``addr`` is a :obj:`bool` (c.f.
+                :func:`~pcapkit.corekit.fields.ipaddress.parse_ip_address`).
+
+        Notes:
+            Through :func:`parse_ip_address` rather than
+            :class:`~ipaddress.IPv4Address`/:class:`~ipaddress.IPv6Address`
+            directly, because :obj:`bool` is an :class:`int` subclass that
+            either constructor accepts without complaint. Before this,
+            ``addr=True`` packed as ``00000001`` (IPv4) or ``::1`` (IPv6) with
+            no exception and no warning at all (c.f. #508, #540).
+
+            The description below uses :attr:`self.__class__.__name__
+            <type.__name__>` rather than :attr:`self.alias
+            <pcapkit.protocols.protocol.Protocol.alias>`, even though the
+            latter is this project's usual choice: :attr:`alias` here reads
+            ``self._acnm``, which :meth:`read` only assigns once the operation
+            code is known, and this method runs from :meth:`make` -- before
+            :meth:`read` has ever executed on a construction-only instance.
+            Using :attr:`alias` would turn every call into an
+            :exc:`AttributeError`, bool or not.
+
         """
         if ptype == Enum_EtherType.Internet_Protocol_version_4:
-            return ipaddress.IPv4Address(addr).packed
+            return cast('IPv4Address', parse_ip_address(
+                addr, f'{self.__class__.__name__}: invalid protocol address', version=4)).packed
         if ptype == Enum_EtherType.Internet_Protocol_version_6:
-            return ipaddress.IPv6Address(addr).packed
+            return cast('IPv6Address', parse_ip_address(
+                addr, f'{self.__class__.__name__}: invalid protocol address', version=6)).packed
 
         if isinstance(addr, str):
             return addr.encode()

pcapkit/protocols/link/ospf.py

@@ -42,6 +42,7 @@
 from pcapkit.const.ospf.authentication import Authentication as Enum_Authentication
 from pcapkit.const.ospf.packet import Packet as Enum_Packet
 from pcapkit.const.reg.transtype import TransType as Enum_TransType
+from pcapkit.corekit.fields.ipaddress import parse_ip_address
 from pcapkit.protocols.data.link.ospf import OSPF as Data_OSPF
 from pcapkit.protocols.data.link.ospf import \
     CrytographicAuthentication as Data_CrytographicAuthentication
@@ -325,8 +326,29 @@ def _make_id_numbers(self, id: 'IPv4Address | str | bytes | bytearray') -> 'byte
         Returns:
             ID bytes.
 
+        Raises:
+            FieldValueError: If ``id`` is a :obj:`bool` (c.f.
+                :func:`~pcapkit.corekit.fields.ipaddress.parse_ip_address`).
+
+        Notes:
+            Latent rather than live: nothing in this module calls this
+            method today (:meth:`make` builds ``router_id``/``area_id``
+            straight from its own arguments), so the only caller is a unit
+            test. It is routed through :func:`parse_ip_address` anyway, so
+            that it does not resurface the defect the moment a caller
+            reaches it -- the same kind of omission is how #481's single-site
+            fix survived to become #491 and then #508 (c.f. #540).
+
+            The description below uses :attr:`self.__class__.__name__
+            <type.__name__>` rather than :attr:`self.alias
+            <pcapkit.protocols.protocol.Protocol.alias>`, because ``alias``
+            here reads ``self._version``, which :meth:`read` only assigns
+            from the wire -- unavailable to a construction-only instance that
+            never went through :meth:`read`.
+
         """
-        return ipaddress.ip_address(id).packed
+        return cast('IPv4Address', parse_ip_address(
+            id, f'{self.__class__.__name__}: invalid ID', version=4)).packed
 
     def _read_encrypt_auth(self, schema: 'Schema_CrytographicAuthentication') -> 'Data_CrytographicAuthentication':
         """Read Authentication field when Cryptographic Authentication is employed,