Skip to content

Commit 32e82ec

Browse files
authored
fix(internet): parse an IGMPv3 Report cut inside its record list (#2650) (#2662)
A Report the capture cuts short falls back to Raw. Report.records is sized by the IGMP length, so past the octets present a GroupRecord consumes nothing, and ListField.unpack refuses it as a loop making no progress. - GroupRecordListField sizes Report.records by the octets present: the record the data ends inside is kept as far as it goes, and Schema.unpack records the short read, as for a cut Query. ListField is unchanged, so its guard against a loop making no progress still holds. - IGMPv3.make takes num_group_records, which IGMPv3._make_data passes through, so a Report cut to fewer records than its count rebuilds as captured. A count that is no int from the records' length to 65535 raises ProtocolError (#2617). - Tests: cuts inside a record header and a source list, a whole Report, cut Additional Data, the field on bytes and a live stream, the count's refusals and edges, the short-read sweep, and TRUNCATED checksum_status. IGMP, schema, field, sweep and doc-claim modules pass; captures parse identically to main.
1 parent d728873 commit 32e82ec

6 files changed

Lines changed: 261 additions & 10 deletions

File tree

‎docs/source/pcapkit/protocols/internet/igmpv3.rst‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@ Header Schemas
8686
:members:
8787
:show-inheritance:
8888

89+
.. autoclass:: pcapkit.protocols.schema.internet.igmpv3.GroupRecordListField
90+
:members:
91+
:show-inheritance:
92+
8993
Data Models
9094
-----------
9195

‎pcapkit/protocols/internet/igmpv3.py‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,7 @@ def make(self,
298298
sources: 'Optional[Iterable[IPv4Address | str | bytes | int]]' = None,
299299
reserved_1: 'int' = 0,
300300
reserved_2: 'bytes' = b'\x00\x00',
301+
num_group_records: 'Optional[int]' = None,
301302
records: 'Optional[Iterable[dict[str, Any] | Data_GroupRecord]]' = None,
302303
payload: 'bytes | ProtocolBase | Schema' = b'',
303304
**kwargs: 'Any') -> 'Schema_IGMPv3':
@@ -347,6 +348,11 @@ def make(self,
347348
(:rfc:`3376#section-4.2.1`); only meaningful for a Report.
348349
reserved_2: The reserved 16 bits before the group-record count
349350
(:rfc:`3376#section-4.2.1`); only meaningful for a Report.
351+
num_group_records: Number of group records, as transmitted on the
352+
wire; the length of ``records`` when not given, so a capture cut
353+
inside its record list rebuilds as captured; only meaningful for
354+
a Report. A count above the length of ``records`` is written as
355+
given, as on a Report the capture cut short.
350356
records: Group records (:rfc:`3376#section-4.2.4`), each either
351357
a :class:`~pcapkit.protocols.data.internet.igmpv3.GroupRecord`
352358
or a :obj:`dict` of the keyword arguments
@@ -360,7 +366,9 @@ def make(self,
360366
361367
Raises:
362368
ProtocolError: If ``max_resp_time`` is not an :obj:`int` from 0
363-
to 255, or ``type`` is neither a Query nor a Report.
369+
to 255, ``num_group_records`` is not an :obj:`int` from the
370+
length of ``records`` to 65535, or ``type`` is neither a Query
371+
nor a Report.
364372
365373
"""
366374
type_value = self._make_index(type, type_default, namespace=type_namespace,
@@ -375,13 +383,19 @@ def make(self,
375383
self._make_group_record(record) if not isinstance(record, dict) else
376384
self._make_group_record(**record) for record in records
377385
]
386+
if num_group_records is None:
387+
num_group_records = len(record_list)
388+
elif isinstance(num_group_records, bool) or not isinstance(num_group_records, int) \
389+
or not len(record_list) <= num_group_records <= 0xFFFF:
390+
raise ProtocolError(f'{self.alias}: invalid num_group_records: {num_group_records!r} '
391+
f'for {len(record_list)} record(s)')
378392
schema = Schema_IGMPv3(
379393
type=type_value,
380394
body=Schema_Report(
381395
reserved_1=reserved_1,
382396
checksum=field,
383397
reserved_2=reserved_2,
384-
num_group_records=len(record_list),
398+
num_group_records=num_group_records,
385399
records=record_list,
386400
payload=payload,
387401
),
@@ -451,6 +465,8 @@ def _make_data(cls, data: 'Data_IGMPv3') -> 'dict[str, Any]': # type: ignore[ov
451465
'reserved_1': report.reserved_1,
452466
'checksum': report.checksum,
453467
'reserved_2': report.reserved_2,
468+
# NOTE: The count as parsed, so a Report cut inside its records rebuilds as captured.
469+
'num_group_records': report.get('num_group_records'),
454470
'records': report.records,
455471
'payload': cls._make_payload(data),
456472
}

‎pcapkit/protocols/schema/internet/igmpv3.py‎

Lines changed: 46 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"""header schema for internet group management protocol version 3"""
44

55
import io
6-
from typing import TYPE_CHECKING
6+
from typing import TYPE_CHECKING, cast
77

88
from pcapkit.const.igmp.record_type import RecordType as Enum_RecordType
99
from pcapkit.const.igmp.type import Type as Enum_Type
@@ -19,7 +19,7 @@
1919

2020
if TYPE_CHECKING:
2121
from ipaddress import IPv4Address
22-
from typing import Any, Optional
22+
from typing import IO, Any, Optional
2323

2424
from typing_extensions import TypedDict
2525

@@ -221,6 +221,46 @@ def __init__(self, record_type: 'Enum_RecordType | int', aux_data_len: 'int',
221221
aux_data: 'bytes') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements
222222

223223

224+
class GroupRecordListField(ListField):
225+
"""Group record list that ends with the data.
226+
227+
:attr:`Report.records` is sized by the IGMP length, which exceeds the
228+
octets present when the capture cuts the Report short. A record read past
229+
them consumes nothing, which :meth:`ListField.unpack
230+
<pcapkit.corekit.fields.collections.ListField.unpack>` refuses to loop on,
231+
so the list is sized by the octets present instead: the record the data
232+
ends inside is kept as far as it goes, and :meth:`Schema.unpack
233+
<pcapkit.protocols.schema.schema.Schema.unpack>` records the short read
234+
(:issue:`2650`).
235+
236+
"""
237+
238+
def unpack(self, buffer: 'bytes | IO[bytes]', packet: 'dict[str, Any]') -> 'list[GroupRecord]':
239+
"""Unpack field value from :obj:`bytes`.
240+
241+
Args:
242+
buffer: Field buffer.
243+
packet: Packet data.
244+
245+
Returns:
246+
Unpacked group records.
247+
248+
"""
249+
if isinstance(buffer, bytes):
250+
present = len(buffer)
251+
else:
252+
start = buffer.tell()
253+
present = buffer.seek(0, io.SEEK_END) - start
254+
buffer.seek(start, io.SEEK_SET)
255+
256+
declared = self._length
257+
self._length = min(declared, present)
258+
try:
259+
return cast('list[GroupRecord]', super().unpack(buffer, packet))
260+
finally:
261+
self._length = declared
262+
263+
224264
@schema_final
225265
class Report(Schema):
226266
"""Header schema for an IGMPv3 Membership Report's body (:rfc:`3376#section-4.2`).
@@ -243,9 +283,10 @@ class Report(Schema):
243283
reserved_2: 'bytes' = BytesField(length=2, default=b'\x00\x00')
244284
#: Number of group records, as received on the wire.
245285
num_group_records: 'int' = UInt16Field()
246-
#: Group records; see :meth:`post_process` for how this is kept to
247-
#: exactly :attr:`num_group_records` entries.
248-
records: 'list[GroupRecord]' = ListField(
286+
#: Group records, read no further than the data goes (see
287+
#: :class:`GroupRecordListField`); see :meth:`post_process` for how this is
288+
#: kept to exactly :attr:`num_group_records` entries.
289+
records: 'list[GroupRecord]' = GroupRecordListField(
249290
length=lambda pkt: pkt['__length__'],
250291
item_type=SchemaField(schema=GroupRecord),
251292
)

‎tests/protocols/internet/test_igmp_checksum_2618_unit.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,9 +148,8 @@ def test_the_checksum_covers_the_whole_ip_payload(self) -> None:
148148
f'0x{whole[2:4].hex()} expected')])
149149

150150
def test_truncated_and_off(self) -> None:
151-
# A Report of IGMPv3 cut short is Raw, its group records unparsed, so the
152-
# IGMPv3 case is a Query.
153-
for name in ('v2-report', 'v3-query'):
151+
# An IGMPv3 Report cut short parses as far as it goes (#2650).
152+
for name in ('v2-report', 'v3-query', 'v3-report'):
154153
packet = message(MESSAGES[name][0][:2], MESSAGES[name][0][4:] + b'XY')
155154
with self.subTest(name):
156155
self.assertEqual(self.status(wire.ipv4(packet, proto=2)[:-1]), (self.Status.TRUNCATED, []))

‎tests/protocols/internet/test_igmp_unit.py‎

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1136,6 +1136,194 @@ def test_make_builds_report_from_dict_records(self) -> None:
11361136
self.assertEqual(schema.body.records[1].num_sources, 1)
11371137

11381138

1139+
#: A Report of two group records with source lists: the first record's header
1140+
#: is octets 8-15 and its sources 16-23, the second's header 24-31 and its
1141+
#: sources 32-43.
1142+
IGMPV3_TWO_RECORD_REPORT = igmpv3_report((
1143+
igmpv3_group_record(1, '239.1.1.1', ('10.0.0.1', '10.0.0.2')),
1144+
igmpv3_group_record(2, '239.2.2.2', ('10.0.0.3', '10.0.0.4', '10.0.0.5')),
1145+
))
1146+
1147+
1148+
class IGMPv3TruncatedReportTests(unittest.TestCase):
1149+
"""A Report the capture cuts inside its record list parses as far as its
1150+
octets go and records the cut, as a cut Query does, rather than falling
1151+
back to ``Raw`` (#2650). Each datagram's IPv4 header declares the whole
1152+
Report, so the IGMP layer is handed fewer octets than its length."""
1153+
1154+
def setUp(self) -> None:
1155+
reimport_once_per_class(self)
1156+
1157+
def parse_cut(self, packet: 'bytes', keep: 'int'):
1158+
"""Parse ``packet`` inside an IPv4 datagram cut to ``keep`` octets of it."""
1159+
from pcapkit.protocols.internet.ipv4 import IPv4
1160+
1161+
with warnings.catch_warnings():
1162+
warnings.simplefilter('ignore')
1163+
outer = IPv4(protocol=2, src='10.0.0.1', dst='224.0.0.22', payload=packet).data
1164+
cut = outer[:len(outer) - len(packet) + keep]
1165+
parsed = IPv4(cut)
1166+
return cut, parsed
1167+
1168+
def assert_rebuilds(self, cut: 'bytes', parsed) -> None:
1169+
"""Both layers rebuild the octets captured, from ``info`` and its dict."""
1170+
from pcapkit.protocols.internet.igmpv3 import IGMPv3
1171+
from pcapkit.protocols.internet.ipv4 import IPv4
1172+
1173+
igmp = parsed.payload
1174+
with warnings.catch_warnings():
1175+
# a cut layer warns that its length runs past the data, which is the point
1176+
warnings.simplefilter('ignore')
1177+
self.assertEqual(IGMPv3.from_data(igmp.info).data, igmp.data)
1178+
self.assertEqual(IGMPv3.from_data(igmp.info.to_dict()).data, igmp.data)
1179+
self.assertEqual(IPv4.from_data(parsed.info).data, cut)
1180+
1181+
def test_whole_report_is_not_marked(self) -> None:
1182+
from pcapkit.protocols.internet.igmpv3 import IGMPv3
1183+
from pcapkit.protocols.misc.no_payload import NoPayload
1184+
1185+
packet = IGMPV3_TWO_RECORD_REPORT
1186+
cut, parsed = self.parse_cut(packet, len(packet))
1187+
igmp = parsed.payload
1188+
1189+
self.assertIsInstance(igmp, IGMPv3)
1190+
self.assertNotIn('__short_read__', igmp.info)
1191+
self.assertEqual(igmp.info.length, len(packet))
1192+
self.assertEqual(
1193+
[(str(record.multicast_address), tuple(str(src) for src in record.sources))
1194+
for record in igmp.info.records],
1195+
[('239.1.1.1', ('10.0.0.1', '10.0.0.2')),
1196+
('239.2.2.2', ('10.0.0.3', '10.0.0.4', '10.0.0.5'))],
1197+
)
1198+
self.assertIsInstance(igmp.payload, NoPayload)
1199+
self.assert_rebuilds(cut, parsed)
1200+
1201+
def test_report_cut_inside_a_record_header(self) -> None:
1202+
"""Cut after the second record's Number of Sources, before its
1203+
Multicast Address: the first record is whole, the second is kept as
1204+
far as it was read."""
1205+
from pcapkit.protocols.internet.igmpv3 import IGMPv3
1206+
from pcapkit.protocols.misc.no_payload import NoPayload
1207+
1208+
packet = IGMPV3_TWO_RECORD_REPORT
1209+
cut, parsed = self.parse_cut(packet, 28)
1210+
igmp = parsed.payload
1211+
1212+
self.assertIsInstance(igmp, IGMPv3)
1213+
self.assertEqual(igmp.info['__short_read__'], ('body', 27))
1214+
self.assertEqual(igmp.info.length, 28)
1215+
self.assertEqual(igmp.data, packet[:28])
1216+
self.assertEqual(igmp.info.num_group_records, 2)
1217+
self.assertEqual(len(igmp.info.records), 2)
1218+
first, second = igmp.info.records
1219+
self.assertEqual(str(first.multicast_address), '239.1.1.1')
1220+
self.assertEqual(tuple(str(src) for src in first.sources), ('10.0.0.1', '10.0.0.2'))
1221+
self.assertEqual(second.record_type, 2)
1222+
self.assertEqual(second.num_sources, 3)
1223+
self.assertIsInstance(igmp.payload, NoPayload)
1224+
self.assert_rebuilds(cut, parsed)
1225+
1226+
def test_report_cut_inside_a_source_list(self) -> None:
1227+
"""Cut inside the first record's second source: one record of the two
1228+
the count declares, which the rebuild still writes as two."""
1229+
from pcapkit.protocols.internet.igmpv3 import IGMPv3
1230+
from pcapkit.protocols.misc.no_payload import NoPayload
1231+
1232+
packet = IGMPV3_TWO_RECORD_REPORT
1233+
cut, parsed = self.parse_cut(packet, 22)
1234+
igmp = parsed.payload
1235+
1236+
self.assertIsInstance(igmp, IGMPv3)
1237+
self.assertEqual(igmp.info['__short_read__'], ('body', 21))
1238+
self.assertEqual(igmp.info.length, 22)
1239+
self.assertEqual(igmp.info.num_group_records, 2)
1240+
self.assertEqual(len(igmp.info.records), 1)
1241+
record = igmp.info.records[0]
1242+
self.assertEqual(str(record.multicast_address), '239.1.1.1')
1243+
self.assertEqual(record.num_sources, 2)
1244+
self.assertEqual(str(record.sources[0]), '10.0.0.1')
1245+
self.assertIsInstance(igmp.payload, NoPayload)
1246+
self.assert_rebuilds(cut, parsed)
1247+
1248+
def test_cut_additional_data_goes_to_the_next_layer(self) -> None:
1249+
"""Octets past the counted records still go to the payload when the
1250+
capture ends inside them (:rfc:`3376#section-4.2.11`)."""
1251+
from pcapkit.protocols.internet.igmpv3 import IGMPv3
1252+
from pcapkit.protocols.misc.raw import Raw
1253+
1254+
packet = igmpv3_report((igmpv3_group_record(1, '224.0.1.1', ('10.0.0.1',)),)) + b'\xca\xfe\xba\xbe'
1255+
cut, parsed = self.parse_cut(packet, len(packet) - 2)
1256+
igmp = parsed.payload
1257+
1258+
self.assertIsInstance(igmp, IGMPv3)
1259+
self.assertEqual(len(igmp.info.records), 1)
1260+
self.assertIsInstance(igmp.payload, Raw)
1261+
self.assertEqual(igmp.payload.info.packet, b'\xca\xfe')
1262+
self.assertEqual(igmp.info.length, len(packet) - 4)
1263+
self.assert_rebuilds(cut, parsed)
1264+
1265+
def test_record_list_reads_bytes_and_a_live_stream_alike(self) -> None:
1266+
"""The record list is sized by the octets present, from a buffer or a
1267+
stream part-way through, and keeps its declared length afterwards."""
1268+
from pcapkit.protocols.schema.internet.igmpv3 import Report
1269+
1270+
records = IGMPV3_TWO_RECORD_REPORT[8:28]
1271+
field = Report.__fields__['records']({'__length__': 36})
1272+
stream = io.BytesIO(b'\xde\xad' + records)
1273+
stream.seek(2)
1274+
1275+
for name, buffer in (('bytes', records), ('stream', stream)):
1276+
with self.subTest(buffer=name), warnings.catch_warnings():
1277+
# the record cut short warns that its length runs past the data
1278+
warnings.simplefilter('ignore')
1279+
parsed = field.unpack(buffer, {})
1280+
self.assertEqual([str(record.multicast_address) for record in parsed],
1281+
['239.1.1.1', '0.0.0.0'])
1282+
self.assertEqual(parsed[1].num_sources, 3)
1283+
self.assertEqual(field.length, 36)
1284+
1285+
1286+
class IGMPv3RecordCountTests(unittest.TestCase):
1287+
"""``IGMPv3.make`` writes the ``num_group_records`` it is given only as an
1288+
:obj:`int` from the length of ``records`` to 65535; any other is refused
1289+
rather than written altered or losing records or payload (#2617, #2650)."""
1290+
1291+
RECORDS = (
1292+
{'record_type': 1, 'multicast_address': '239.1.1.1'},
1293+
{'record_type': 2, 'multicast_address': '239.2.2.2'},
1294+
)
1295+
1296+
def setUp(self) -> None:
1297+
reimport_once_per_class(self)
1298+
1299+
def test_a_count_the_records_or_the_field_cannot_carry_is_refused(self) -> None:
1300+
import re
1301+
1302+
from pcapkit.const.igmp.type import Type
1303+
from pcapkit.protocols.internet.igmpv3 import IGMPv3
1304+
from pcapkit.utilities.exceptions import ProtocolError
1305+
1306+
proto = object.__new__(IGMPv3)
1307+
for count, payload in ((65536, b''), (-1, b''), (1, b'XYZW'), (0, b'XYZW'),
1308+
(True, b''), (2.0, b''), ('2', b'')):
1309+
with self.subTest(count=count, payload=payload), self.assertRaisesRegex(
1310+
ProtocolError, rf'^IGMPv3: invalid num_group_records: {re.escape(repr(count))} for 2 record'):
1311+
proto.make(type=Type.IGMPv3_Membership_Report, records=self.RECORDS,
1312+
num_group_records=count, payload=payload)
1313+
1314+
def test_a_count_from_the_records_to_the_field_maximum_is_written(self) -> None:
1315+
from pcapkit.const.igmp.type import Type
1316+
from pcapkit.protocols.internet.igmpv3 import IGMPv3
1317+
1318+
records = igmpv3_group_record(1, '239.1.1.1') + igmpv3_group_record(2, '239.2.2.2')
1319+
for count, wire in ((None, 2), (2, 2), (3, 3), (0xFFFF, 0xFFFF)):
1320+
with self.subTest(count=count):
1321+
built = IGMPv3(type=Type.IGMPv3_Membership_Report, records=self.RECORDS,
1322+
num_group_records=count, payload=b'XYZW').data
1323+
self.assertEqual(built[6:8], wire.to_bytes(2, 'big'))
1324+
self.assertEqual(built[8:], records + b'XYZW')
1325+
1326+
11391327
class IGMPDispatchTests(unittest.TestCase):
11401328
"""The IP protocol 2 registry entry is the abstract base, which picks the
11411329
version class per message (#2439, #2448)."""

‎tests/protocols/test_short_read_roundtrip_unit.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@ def _ipv6(code: int, body: bytes) -> bytes:
7373
('ospf', 'application.ospf:OSPF', {}, bytes.fromhex('020100180102030400000000abcd0000') + bytes(8)),
7474
('ipx', 'internet.ipx:IPX', {}, bytes.fromhex('ffff001e0000000000000101010101010451000000000202020202020452')),
7575
('ipv4', 'internet.ipv4:IPv4', {}, bytes.fromhex('450000141234000040fd7b83c0000201c6336401')),
76+
('igmpv3-report', 'internet.ipv4:IPv4', {}, bytes.fromhex(
77+
'45000040123400000102bd710a000001e0000016' '2200c7e100000002'
78+
'01000002ef0101010a0000010a000002' '02000003ef0202020a0000030a0000040a000005')),
7679
('ipv6', IPV6, {}, _ipv6(59, b'')),
7780
('ipv6-chain', IPV6, {}, bytes.fromhex(
7881
'600000000020004000000000000000000000000000000001000000000000000000000000000000023c'

0 commit comments

Comments
 (0)