diff --git a/pcapkit/protocols/internet/hopopt.py b/pcapkit/protocols/internet/hopopt.py index 5391e8e15d..48b14ecb0e 100644 --- a/pcapkit/protocols/internet/hopopt.py +++ b/pcapkit/protocols/internet/hopopt.py @@ -755,7 +755,9 @@ def _read_opt_calipso(self, schema: 'Schema_CALIPSOOption', *, options: 'Option' """ if schema.len < 8 and schema.len % 8 != 0: raise ProtocolError(f'{self.alias}: [OptNo {schema.type}] invalid format') - if schema.cmpt_len % 2 != 0: + # ``Cmpt Length`` counts 32-bit words and may be odd [:rfc:`5570#section-5.2`]; + # it only has to fit inside ``Opt Data Len`` + if schema.len < 8 + schema.cmpt_len * 4: raise ProtocolError(f'{self.alias}: [OptNo {schema.type}] invalid format') opt = Data_CALIPSOOption( @@ -1584,9 +1586,9 @@ def _make_opt_calipso(self, code: 'Enum_Option', opt: 'Optional[Data_CALIPSOOpti domain: CALIPSO domain of interpretation level: sensitivity level checksum: checksum of the option - bitmap: compartment bitmap, as :obj:`bytes` or as the - :obj:`tuple` of octets the reader stores; its length must be a - multiple of 4 octets + bitmap: compartment bitmap, as a bytes-like object or as an + iterable of octets, such as the :obj:`tuple` the reader stores; + its length must be a multiple of 4 octets pad: octets after the compartment bitmap, counted in ``Opt Data Len`` **kwargs: arbitrary keyword arguments @@ -1594,7 +1596,10 @@ def _make_opt_calipso(self, code: 'Enum_Option', opt: 'Optional[Data_CALIPSOOpti Constructed option schema. Raises: - ProtocolError: If the bitmap is not a whole number of 32-bit words. + ProtocolError: If the bitmap is not bytes-like or an iterable of + octets, or is not a whole number of 32-bit words, if ``pad`` is not :obj:`bytes`, + or if the option data would exceed the 255 octets + ``Opt Data Len`` can count. Note: ``Cmpt Length`` on the wire counts 32-bit words [:rfc:`5570#section-5.1`], @@ -1607,10 +1612,34 @@ def _make_opt_calipso(self, code: 'Enum_Option', opt: 'Optional[Data_CALIPSOOpti checksum = opt.checksum bitmap = opt.cmpt_bitmap if hasattr(opt, 'cmpt_bitmap') else None pad = opt.pad - bitmap_bytes = bytes(bitmap) if bitmap is not None else b'' + if bitmap is None: + bitmap_bytes = b'' + elif isinstance(bitmap, (int, str)): + raise ProtocolError(f'{self.alias}: [OptNo {code}] invalid CALIPSO bitmap type: ' + f'{type(bitmap).__name__}') + else: + try: # bytes-like object + bitmap_bytes = bytes(memoryview(bitmap)) + except TypeError: # iterable of octets + try: + octets = tuple(bitmap) + except TypeError as exc: + raise ProtocolError(f'{self.alias}: [OptNo {code}] invalid CALIPSO bitmap type: ' + f'{type(bitmap).__name__}') from exc + for octet in octets: + if isinstance(octet, bool) or not isinstance(octet, int) or not 0 <= octet <= 255: + raise ProtocolError(f'{self.alias}: [OptNo {code}] invalid CALIPSO bitmap ' + f'octet: {octet!r}') + bitmap_bytes = bytes(octets) if len(bitmap_bytes) % 4 != 0: raise ProtocolError(f'{self.alias}: [OptNo {code}] invalid CALIPSO bitmap length: ' f'{len(bitmap_bytes)}') + if not isinstance(pad, (bytes, bytearray)): + raise ProtocolError(f'{self.alias}: [OptNo {code}] invalid CALIPSO pad type: ' + f'{type(pad).__name__}') + if 8 + len(bitmap_bytes) + len(pad) > 255: + raise ProtocolError(f'{self.alias}: [OptNo {code}] too long CALIPSO option data: ' + f'{8 + len(bitmap_bytes) + len(pad)}') return Schema_CALIPSOOption( type=code, diff --git a/pcapkit/protocols/internet/ipv6_opts.py b/pcapkit/protocols/internet/ipv6_opts.py index e8929cb655..4d349e7095 100644 --- a/pcapkit/protocols/internet/ipv6_opts.py +++ b/pcapkit/protocols/internet/ipv6_opts.py @@ -744,7 +744,9 @@ def _read_opt_calipso(self, schema: 'Schema_CALIPSOOption', *, options: 'Option' """ if schema.len < 8 and schema.len % 8 != 0: raise ProtocolError(f'{self.alias}: [OptNo {schema.type}] invalid format') - if schema.cmpt_len % 2 != 0: + # ``Cmpt Length`` counts 32-bit words and may be odd [:rfc:`5570#section-5.2`]; + # it only has to fit inside ``Opt Data Len`` + if schema.len < 8 + schema.cmpt_len * 4: raise ProtocolError(f'{self.alias}: [OptNo {schema.type}] invalid format') opt = Data_CALIPSOOption( @@ -1573,9 +1575,9 @@ def _make_opt_calipso(self, code: 'Enum_Option', opt: 'Optional[Data_CALIPSOOpti domain: CALIPSO domain of interpretation level: sensitivity level checksum: checksum of the option - bitmap: compartment bitmap, as :obj:`bytes` or as the - :obj:`tuple` of octets the reader stores; its length must be a - multiple of 4 octets + bitmap: compartment bitmap, as a bytes-like object or as an + iterable of octets, such as the :obj:`tuple` the reader stores; + its length must be a multiple of 4 octets pad: octets after the compartment bitmap, counted in ``Opt Data Len`` **kwargs: arbitrary keyword arguments @@ -1583,7 +1585,10 @@ def _make_opt_calipso(self, code: 'Enum_Option', opt: 'Optional[Data_CALIPSOOpti Constructed option schema. Raises: - ProtocolError: If the bitmap is not a whole number of 32-bit words. + ProtocolError: If the bitmap is not bytes-like or an iterable of + octets, or is not a whole number of 32-bit words, if ``pad`` is not :obj:`bytes`, + or if the option data would exceed the 255 octets + ``Opt Data Len`` can count. Note: ``Cmpt Length`` on the wire counts 32-bit words [:rfc:`5570#section-5.1`], @@ -1596,10 +1601,34 @@ def _make_opt_calipso(self, code: 'Enum_Option', opt: 'Optional[Data_CALIPSOOpti checksum = opt.checksum bitmap = opt.cmpt_bitmap if hasattr(opt, 'cmpt_bitmap') else None pad = opt.pad - bitmap_bytes = bytes(bitmap) if bitmap is not None else b'' + if bitmap is None: + bitmap_bytes = b'' + elif isinstance(bitmap, (int, str)): + raise ProtocolError(f'{self.alias}: [OptNo {code}] invalid CALIPSO bitmap type: ' + f'{type(bitmap).__name__}') + else: + try: # bytes-like object + bitmap_bytes = bytes(memoryview(bitmap)) + except TypeError: # iterable of octets + try: + octets = tuple(bitmap) + except TypeError as exc: + raise ProtocolError(f'{self.alias}: [OptNo {code}] invalid CALIPSO bitmap type: ' + f'{type(bitmap).__name__}') from exc + for octet in octets: + if isinstance(octet, bool) or not isinstance(octet, int) or not 0 <= octet <= 255: + raise ProtocolError(f'{self.alias}: [OptNo {code}] invalid CALIPSO bitmap ' + f'octet: {octet!r}') + bitmap_bytes = bytes(octets) if len(bitmap_bytes) % 4 != 0: raise ProtocolError(f'{self.alias}: [OptNo {code}] invalid CALIPSO bitmap length: ' f'{len(bitmap_bytes)}') + if not isinstance(pad, (bytes, bytearray)): + raise ProtocolError(f'{self.alias}: [OptNo {code}] invalid CALIPSO pad type: ' + f'{type(pad).__name__}') + if 8 + len(bitmap_bytes) + len(pad) > 255: + raise ProtocolError(f'{self.alias}: [OptNo {code}] too long CALIPSO option data: ' + f'{8 + len(bitmap_bytes) + len(pad)}') return Schema_CALIPSOOption( type=code, diff --git a/tests/protocols/internet/test_ipv6_calipso_make_validation_unit.py b/tests/protocols/internet/test_ipv6_calipso_make_validation_unit.py new file mode 100644 index 0000000000..f7b7ecfbb1 --- /dev/null +++ b/tests/protocols/internet/test_ipv6_calipso_make_validation_unit.py @@ -0,0 +1,142 @@ +# -*- coding: utf-8 -*- +"""Constructing a CALIPSO option validates its data length and its pad. + +GitHub issue #1408: ``Opt Data Len`` is one octet, but ``make()`` wrote it +unchecked, so a bitmap plus pad over 255 octets produced a corrupt length +octet. A pad that was not :obj:`bytes`, or a bitmap that was neither +bytes-like nor an iterable of octets, surfaced as a raw :exc:`struct.error`, :exc:`TypeError` or +:exc:`ValueError`. All of these now raise +:exc:`~pcapkit.utilities.exceptions.ProtocolError`. ``make()`` also built an +odd ``Cmpt Length``, which RFC 5570 permits, but the reader rejected it; the +reader now accepts it. + +The classes are imported inside each test, after +:func:`~tests._support.reimport_once_per_class`, so that they belong to the +:mod:`pcapkit` import that is live when the test runs. + +""" + +import importlib +import io +import unittest + +from tests._support import reimport_once_per_class + +#: The protocol classes sharing the option code, by module and class name. +PROTOCOLS = ( + ('pcapkit.protocols.internet.hopopt', 'HOPOPT'), + ('pcapkit.protocols.internet.ipv6_opts', 'IPv6_Opts'), +) + + +class TestCALIPSOMakeValidation(unittest.TestCase): + """Pin the CALIPSO ``make()`` length and pad checks.""" + + def setUp(self) -> None: + reimport_once_per_class(self) + + def _protocols(self) -> 'list[type]': + return [getattr(importlib.import_module(mod), name) for mod, name in PROTOCOLS] + + def _make(self, cls: 'type', **fields: 'object') -> 'object': + from pcapkit.const.ipv6.option import Option + + return cls(next=59, options=[(Option.CALIPSO, dict(domain=1, level=5, **fields))]) + + def test_overlong_option_data_raises(self) -> None: + from pcapkit.utilities.exceptions import ProtocolError + + cases = { + 'pad 248': {'pad': b'\x00' * 248}, + 'pad 1000': {'pad': b'\x00' * 1000}, + 'bitmap 248': {'bitmap': b'\x00' * 248}, + 'bitmap 240 + pad 8': {'bitmap': b'\x00' * 240, 'pad': b'\x00' * 8}, + } + for cls in self._protocols(): + for label, fields in cases.items(): + with self.subTest(protocol=cls.__name__, case=label): + with self.assertRaisesRegex(ProtocolError, 'too long CALIPSO option data'): + self._make(cls, **fields) + + def test_largest_option_data_round_trips(self) -> None: + from pcapkit.const.ipv6.option import Option + + for cls in self._protocols(): + for label, fields in { + 'pad 247': {'pad': b'\xaa' * 247}, + 'bitmap 240 + pad 7': {'bitmap': b'\x11' * 240, 'pad': b'\xaa' * 7}, + }.items(): + with self.subTest(protocol=cls.__name__, case=label): + made = self._make(cls, **fields) + data = made.data # type: ignore[attr-defined] + self.assertEqual(data[2:4], b'\x07\xff') + parsed = cls(io.BytesIO(data), len(data), extension=True) + self.assertEqual(parsed.info.options[Option.CALIPSO].pad, fields['pad']) # type: ignore[attr-defined] + self.assertEqual(cls.from_data(parsed.info).data, data) + + def test_non_bytes_pad_raises(self) -> None: + from pcapkit.utilities.exceptions import ProtocolError + + for cls in self._protocols(): + for pad in ('abcd', [0, 0, 0, 0], None, 4): + with self.subTest(protocol=cls.__name__, pad=pad): + with self.assertRaisesRegex(ProtocolError, 'invalid CALIPSO pad type'): + self._make(cls, pad=pad) + + def test_bad_bitmap_raises(self) -> None: + from pcapkit.utilities.exceptions import ProtocolError + + cases = { + 'element over 255': ((256, 0, 0, 0), 'invalid CALIPSO bitmap octet'), + 'negative element': ((-1, 0, 0, 0), 'invalid CALIPSO bitmap octet'), + 'str element': (('a', 0, 0, 0), 'invalid CALIPSO bitmap octet'), + 'bool element': ((True, 0, 0, 0), 'invalid CALIPSO bitmap octet'), + 'str bitmap': ('abcd', 'invalid CALIPSO bitmap type'), + 'int bitmap': (4, 'invalid CALIPSO bitmap type'), + 'bool bitmap': (True, 'invalid CALIPSO bitmap type'), + 'non-iterable bitmap': (4.0, 'invalid CALIPSO bitmap type'), + } + for cls in self._protocols(): + for label, (bitmap, message) in cases.items(): + with self.subTest(protocol=cls.__name__, case=label): + with self.assertRaisesRegex(ProtocolError, message): + self._make(cls, bitmap=bitmap) + + def test_odd_cmpt_length_round_trips(self) -> None: + # RFC 5570, section 5.2: the bitmap is sized in 32-bit words, so an + # odd ``Cmpt Length`` (here 1 and 3) is valid and has to re-parse + from pcapkit.const.ipv6.option import Option + + for cls in self._protocols(): + for words in (1, 3): + bitmap = bytes(range(1, 4 * words + 1)) + with self.subTest(protocol=cls.__name__, cmpt_len=words): + data = self._make(cls, bitmap=bitmap).data # type: ignore[attr-defined] + self.assertEqual(data[4 + 4], words) + parsed = cls(io.BytesIO(data), len(data), extension=True) + calipso = parsed.info.options[Option.CALIPSO] # type: ignore[attr-defined] + self.assertEqual(calipso.cmpt_len, 4 * words) + self.assertEqual(bytes(calipso.cmpt_bitmap), bitmap) + self.assertEqual(cls.from_data(parsed.info).data, data) + + def test_bitmap_sequences_match_bytes(self) -> None: + for cls in self._protocols(): + expected = self._make(cls, bitmap=b'\x01\x02\x03\x04\x05\x06\x07\x08').data # type: ignore[attr-defined] + for bitmap in (bytearray(range(1, 9)), memoryview(bytes(range(1, 9))), + tuple(range(1, 9)), list(range(1, 9)), range(1, 9), + (octet for octet in range(1, 9))): + with self.subTest(protocol=cls.__name__, bitmap=type(bitmap).__name__): + self.assertEqual(self._make(cls, bitmap=bitmap).data, expected) # type: ignore[attr-defined] + with self.subTest(protocol=cls.__name__, bitmap='memoryview zeros'): + self.assertEqual(self._make(cls, bitmap=memoryview(bytes(8))).data, # type: ignore[attr-defined] + self._make(cls, bitmap=b'\x00' * 8).data) # type: ignore[attr-defined] + + def test_bytearray_pad_matches_bytes(self) -> None: + for cls in self._protocols(): + with self.subTest(protocol=cls.__name__): + self.assertEqual(self._make(cls, pad=bytearray(b'\xaa\xbb\xcc\xdd')).data, # type: ignore[attr-defined] + self._make(cls, pad=b'\xaa\xbb\xcc\xdd').data) # type: ignore[attr-defined] + + +if __name__ == '__main__': + unittest.main()