From d0c397767abe3b98191e4d3ecf6ee45ea9e5e5d1 Mon Sep 17 00:00:00 2001 From: Gijs Molenaar Date: Thu, 8 Oct 2026 07:06:29 +0200 Subject: [PATCH 1/2] fix: reject USERDATA last-data-unit values other than 0x00 and 0x01 (#943) Co-Authored-By: Claude Sonnet 5.5 --- CHANGES.md | 2 ++ snap7/s7protocol.py | 4 +++- tests/test_multipacket.py | 10 ++++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 8a4f1f95..60e03268 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -15,6 +15,8 @@ Unreleased * `get_string` and `get_wstring` now raise `ValueError` when the buffer is shorter than the declared current length, instead of silently returning a truncated value (#945). +* USERDATA responses whose last-data-unit byte is neither `0x00` nor `0x01` are rejected with `S7ProtocolError` + instead of being treated as "more data" (#943). * `get_wstring` compared the declared maximum in bytes with the 16382-character limit, so any WSTRING with a capacity of 8192 characters or more was rejected, including ones written by `set_wstring` (#923). * The get-block-info request now places the trailing `A` after the block number, as native Snap7 does diff --git a/snap7/s7protocol.py b/snap7/s7protocol.py index 22707588..a090e3b6 100644 --- a/snap7/s7protocol.py +++ b/snap7/s7protocol.py @@ -1460,6 +1460,8 @@ def check_userdata_response( if param_error != 0: error_msg = get_protocol_error_message(param_error) raise S7ProtocolError(f"USERDATA request failed: {error_msg} (0x{param_error:04x})") + if params.get("last_data_unit", 0) not in (0x00, 0x01): + raise S7ProtocolError(f"Invalid USERDATA last data unit value 0x{params['last_data_unit']:02x}") if expected_group is not None and params.get("group") != expected_group: raise S7ProtocolError("Unexpected USERDATA response function group") if expected_subfunction is not None and params.get("subfunction") != expected_subfunction: @@ -1689,7 +1691,7 @@ def _parse_userdata_response_params(self, param_data: bytes) -> Dict[str, Any]: [6] Subfunction [7] Sequence number (used as DataRef in follow-up) [8] Data unit reference - [9] Last data unit (0x00 = last, non-zero = more) + [9] Last data unit (0x00 = last, 0x01 = more) [10-11] Error code """ type_group = param_data[5] diff --git a/tests/test_multipacket.py b/tests/test_multipacket.py index 1a982f72..59d403e2 100644 --- a/tests/test_multipacket.py +++ b/tests/test_multipacket.py @@ -442,3 +442,13 @@ def test_tpdu_size_in_cotp_cr(self) -> None: conn = ISOTCPConnection("127.0.0.1", tpdu_size=TPDUSize.S_2048) cr_pdu = conn._build_cotp_cr() assert cr_pdu[-3:] == bytes([0xC0, 0x01, TPDUSize.S_2048]) + + +@pytest.mark.parametrize("last", [0x02, 0xFF]) +def test_userdata_invalid_last_data_unit_rejected(last: int) -> None: + """Only 0x00 and 0x01 are valid last-data-unit values.""" + params = bytes([0, 1, 0x12, 8, 0x12, 0x84, 1, 1, 0, last, 0, 0]) + data = bytes([0xFF, 9, 0, 4]) + b"abcd" + pdu = struct.pack(">BBHHHH", 0x32, 7, 0, 1, len(params), len(data)) + params + data + with pytest.raises(S7ProtocolError): + S7Protocol().parse_response(pdu) From c908b5807450a074b86c10a02e37646917b8bd9a Mon Sep 17 00:00:00 2001 From: Gijs Molenaar Date: Thu, 8 Oct 2026 09:11:30 +0200 Subject: [PATCH 2/2] docs: use PR number in changelog entry --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 60e03268..f0b5d30d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -16,7 +16,7 @@ Unreleased * `get_string` and `get_wstring` now raise `ValueError` when the buffer is shorter than the declared current length, instead of silently returning a truncated value (#945). * USERDATA responses whose last-data-unit byte is neither `0x00` nor `0x01` are rejected with `S7ProtocolError` - instead of being treated as "more data" (#943). + instead of being treated as "more data" (#946). * `get_wstring` compared the declared maximum in bytes with the 16382-character limit, so any WSTRING with a capacity of 8192 characters or more was rejected, including ones written by `set_wstring` (#923). * The get-block-info request now places the trailing `A` after the block number, as native Snap7 does