From 9fd05a10443b4a652d1651ffd71d7874eaa63258 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Sun, 25 Oct 2026 23:34:51 -0400 Subject: [PATCH] fix(ethernet,pcapng): reject a non-string MAC or EUI address (#2222) Problem: Ethernet._make_mac_addr, PCAPNG._make_mac_addr and PCAPNG._make_eui_addr passed a bool, int, None or float to the address pattern's fullmatch, which raised a bare TypeError that neither ProtocolError nor ValueError catches. Fix: anything not str/bytes/bytearray, bool included, raises ProtocolError. Every message in the three makers carries the protocol prefix ("Ethernet: ...", "PCAP-NG: ..."). Test: test_ethernet_mac_type_unit.py (dst/src) and test_pcapng_addr_type_2222_unit.py (IDB if_MACaddr/if_EUIaddr options) pin the message for bool/int/None/float and for a malformed string, and that str/bytes/bytearray still pack. --- pcapkit/protocols/link/ethernet.py | 10 ++- pcapkit/protocols/misc/pcapng.py | 20 +++-- .../link/test_ethernet_mac_type_unit.py | 51 +++++++++++ .../misc/test_pcapng_addr_type_2222_unit.py | 86 +++++++++++++++++++ 4 files changed, 158 insertions(+), 9 deletions(-) create mode 100644 tests/protocols/link/test_ethernet_mac_type_unit.py create mode 100644 tests/protocols/misc/test_pcapng_addr_type_2222_unit.py diff --git a/pcapkit/protocols/link/ethernet.py b/pcapkit/protocols/link/ethernet.py index ae1525399d..4c51710b3e 100644 --- a/pcapkit/protocols/link/ethernet.py +++ b/pcapkit/protocols/link/ethernet.py @@ -232,12 +232,16 @@ def _make_mac_addr(self, addr: 'str | bytes | bytearray') -> 'bytes': MAC address packed as 6 octets. Raises: - ProtocolError: If ``addr`` is not a ``:``- or ``-``-separated - *hex* encoded MAC address. + ProtocolError: If ``addr`` is not a :class:`str`, :class:`bytes` + or :class:`bytearray` (a :obj:`bool` included), or is not a + ``:``- or ``-``-separated *hex* encoded MAC address. """ + if not isinstance(addr, (str, bytes, bytearray)): + raise ProtocolError(f'{self.alias}: invalid MAC address: must be a str, bytes or ' + f'bytearray, not {addr!r}') _addr = addr.encode() if isinstance(addr, str) else addr if PAT_MAC_ADDR.fullmatch(_addr) is not None: return bytes.fromhex(_addr.replace(b':', b'').replace(b'-', b'').decode()) - raise ProtocolError(f'invalid MAC address: {addr!r}') + raise ProtocolError(f'{self.alias}: invalid MAC address: {addr!r}') diff --git a/pcapkit/protocols/misc/pcapng.py b/pcapkit/protocols/misc/pcapng.py index 5715640e13..052fec224b 100644 --- a/pcapkit/protocols/misc/pcapng.py +++ b/pcapkit/protocols/misc/pcapng.py @@ -2118,15 +2118,19 @@ def _make_mac_addr(self, addr: 'str | bytes | bytearray') -> 'bytes': MAC address packed as 6 octets. Raises: - ProtocolError: If ``addr`` is not a ``:``- or ``-``-separated - *hex* encoded MAC address. + ProtocolError: If ``addr`` is not a :class:`str`, :class:`bytes` + or :class:`bytearray` (a :obj:`bool` included), or is not a + ``:``- or ``-``-separated *hex* encoded MAC address. """ + if not isinstance(addr, (str, bytes, bytearray)): + raise ProtocolError(f'PCAP-NG: invalid MAC address: must be a str, bytes or ' + f'bytearray, not {addr!r}') _addr = addr.encode() if isinstance(addr, str) else addr if PAT_MAC_ADDR.fullmatch(_addr) is not None: return bytes.fromhex(_addr.replace(b':', b'').replace(b'-', b'').decode()) - raise ProtocolError(f'invalid MAC address: {addr!r}') + raise ProtocolError(f'PCAP-NG: invalid MAC address: {addr!r}') def _make_eui_addr(self, addr: 'str | bytes | bytearray') -> 'bytes': """Make EUI address. @@ -2138,15 +2142,19 @@ def _make_eui_addr(self, addr: 'str | bytes | bytearray') -> 'bytes': EUI address packed as 8 octets. Raises: - ProtocolError: If ``addr`` is not a ``:``- or ``-``-separated - *hex* encoded EUI-64 address. + ProtocolError: If ``addr`` is not a :class:`str`, :class:`bytes` + or :class:`bytearray` (a :obj:`bool` included), or is not a + ``:``- or ``-``-separated *hex* encoded EUI-64 address. """ + if not isinstance(addr, (str, bytes, bytearray)): + raise ProtocolError(f'PCAP-NG: invalid EUI address: must be a str, bytes or ' + f'bytearray, not {addr!r}') _addr = addr.encode() if isinstance(addr, str) else addr if PAT_EUI_ADDR.fullmatch(_addr) is not None: return bytes.fromhex(_addr.replace(b':', b'').replace(b'-', b'').decode()) - raise ProtocolError(f'invalid EUI address: {addr!r}') + raise ProtocolError(f'PCAP-NG: invalid EUI address: {addr!r}') def _decode_next_layer(self, dict_: 'Data_PCAPNG', proto: 'Optional[int]' = None, length: 'Optional[int]' = None, *, packet: 'Optional[dict[str, Any]]' = None) -> 'Data_PCAPNG': # pylint: disable=arguments-differ diff --git a/tests/protocols/link/test_ethernet_mac_type_unit.py b/tests/protocols/link/test_ethernet_mac_type_unit.py new file mode 100644 index 0000000000..0682ed81c2 --- /dev/null +++ b/tests/protocols/link/test_ethernet_mac_type_unit.py @@ -0,0 +1,51 @@ +# -*- coding: utf-8 -*- +"""Ethernet rejects a non-string MAC address with an in-library error. + +GitHub issue #2222: :meth:`Ethernet._make_mac_addr +` passed a +:obj:`bool`, :class:`int`, :obj:`None` or :class:`float` straight to the MAC +pattern, which raised a bare :class:`TypeError` that neither +:class:`~pcapkit.utilities.exceptions.ProtocolError` nor :class:`ValueError` +catches. Anything not a :class:`str`, :class:`bytes` or :class:`bytearray`, +a :obj:`bool` included, raises +:class:`~pcapkit.utilities.exceptions.ProtocolError`. + +Every case builds its own frame in memory and reads no capture. + +""" + +import unittest + +from pcapkit.protocols.link.ethernet import Ethernet +from pcapkit.utilities.exceptions import ProtocolError + + +class TestEthernetMacAddressType(unittest.TestCase): + """Pin the error raised for each non-string MAC address type.""" + + def test_non_string_raises_protocol_error(self) -> None: + for field in ('dst', 'src'): + for value in (True, False, 5, None, 1.5): + with self.subTest(field=field, value=value): + with self.assertRaises(ProtocolError) as context: + Ethernet(**{field: value}) + self.assertEqual( + str(context.exception), + f'Ethernet: invalid MAC address: must be a str, bytes or bytearray, ' + f'not {value!r}') + + def test_malformed_string_message(self) -> None: + with self.assertRaises(ProtocolError) as context: + Ethernet(dst='not-a-mac') + self.assertEqual(str(context.exception), "Ethernet: invalid MAC address: 'not-a-mac'") + + def test_supported_forms_still_pack(self) -> None: + raw = bytes.fromhex('0123456789ab') + for value in ('01:23:45:67:89:ab', b'01-23-45-67-89-ab', + bytearray(b'01:23:45:67:89:ab')): + with self.subTest(value=value): + self.assertEqual(Ethernet(dst=value).data[:6], raw) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/protocols/misc/test_pcapng_addr_type_2222_unit.py b/tests/protocols/misc/test_pcapng_addr_type_2222_unit.py new file mode 100644 index 0000000000..f74f414b60 --- /dev/null +++ b/tests/protocols/misc/test_pcapng_addr_type_2222_unit.py @@ -0,0 +1,86 @@ +# -*- coding: utf-8 -*- +"""GitHub issue #2222: PCAP-NG rejects a non-string MAC or EUI address. + +:meth:`PCAPNG._make_mac_addr +` and +:meth:`PCAPNG._make_eui_addr +` passed a :obj:`bool`, +:class:`int`, :obj:`None` or :class:`float` straight to the address pattern, +which raised a bare :class:`TypeError`. Anything not a :class:`str`, +:class:`bytes` or :class:`bytearray`, a :obj:`bool` included, raises +:class:`~pcapkit.utilities.exceptions.ProtocolError`. +Each case reaches the maker through the ``if_MACaddr`` or ``if_EUIaddr`` +option of an Interface Description Block. + +:mod:`pcapkit` is imported inside each test, after +:func:`~tests._support.reimport_once_per_class`. + +""" + +import unittest +from typing import TYPE_CHECKING + +from tests.protocols.misc.test_pcapng_roundtrip2_unit import PCAPNGTestCase + +if TYPE_CHECKING: + from typing import Any + + +class TestPCAPNGAddressType(PCAPNGTestCase): + """Pin the error raised for each non-string ``if_MACaddr``/``if_EUIaddr`` value.""" + + def _idb(self, option: 'str', interface: 'Any') -> 'Any': + """Build an Interface Description Block carrying one address option.""" + from pcapkit.const.pcapng.block_type import BlockType + from pcapkit.const.pcapng.option_type import OptionType + from pcapkit.const.reg.linktype import LinkType + from pcapkit.protocols.misc.pcapng import PCAPNG + + return PCAPNG(num=2, sct=1, ctx=self._context(), type=BlockType.Interface_Description_Block, + block={'linktype': LinkType.ETHERNET, 'snaplen': 65535, + 'options': [(OptionType[option], {'interface': interface})]}) + + OPTIONS = ('if_MACaddr', 'if_EUIaddr') + + #: Address kind each option's error message names. + KINDS = {'if_MACaddr': 'MAC', 'if_EUIaddr': 'EUI'} + + def test_non_string_raises_protocol_error(self) -> None: + from pcapkit.utilities.exceptions import ProtocolError + + for option in self.OPTIONS: + for value in (True, False, 5, None, 1.5): + with self.subTest(option=option, value=value): + with self.assertRaises(ProtocolError) as context: + self._idb(option, value) + self.assertEqual( + str(context.exception), + f'PCAP-NG: invalid {self.KINDS[option]} address: must be a str, bytes ' + f'or bytearray, not {value!r}') + + def test_malformed_string_message(self) -> None: + from pcapkit.utilities.exceptions import ProtocolError + + for option in self.OPTIONS: + with self.subTest(option=option): + with self.assertRaises(ProtocolError) as context: + self._idb(option, 'bad') + self.assertEqual(str(context.exception), + f"PCAP-NG: invalid {self.KINDS[option]} address: 'bad'") + + def test_supported_forms_still_pack(self) -> None: + from pcapkit.const.pcapng.option_type import OptionType + + cases = { + 'if_MACaddr': ('01:23:45:67:89:ab', '01-23-45-67-89-ab'), + 'if_EUIaddr': ('02:34:56:ff:fe:78:9a:bc', '02-34-56-ff-fe-78-9a-bc'), + } + for option, (text, dashed) in cases.items(): + for value in (text, dashed.encode(), bytearray(text.encode())): + with self.subTest(option=option, value=value): + block = self._idb(option, value) + self.assertEqual(block.info.options[OptionType[option]].interface, text) + + +if __name__ == '__main__': + unittest.main()