Skip to content

Commit 46ebb76

Browse files
committed
Validate FC23 write count against MAX_WRITE_COUNT
ReadWriteMultipleRegistersRequest.encode() checked the write count against MAX_READ_COUNT (125) rather than MAX_WRITE_COUNT (121), so the client could encode a request larger than the 253 byte MODBUS PDU maximum. MAX_WRITE_COUNT had no other reference in the repo. decode() keeps MAX_READ_COUNT on purpose: raising there makes DecodePDU.decode() drop the frame, so the server would answer nothing instead of ILLEGAL_VALUE.
1 parent 7843881 commit 46ebb76

2 files changed

Lines changed: 30 additions & 1 deletion

File tree

pymodbus/pdu/register_message.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ def encode(self) -> bytes:
131131
self.verifyAddress(address=self.read_address)
132132
self.verifyAddress(address=self.write_address)
133133
self.verifyCount(self.MAX_READ_COUNT, count=self.read_count)
134-
self.verifyCount(self.MAX_READ_COUNT, count=self.write_count)
134+
self.verifyCount(self.MAX_WRITE_COUNT, count=self.write_count)
135135
result = struct.pack(
136136
">HHHHB",
137137
self.read_address,

test/pdu/test_register_read_messages.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Test register read messages."""
22

3+
import struct
34
from unittest import mock
45

56
import pytest
@@ -84,6 +85,34 @@ def test_register_read_response_decode_error(self):
8485
reg.decode(b"\x14\x00\x03\x00\x11")
8586
assert exc_info.value.fcode == reg.function_code
8687

88+
def test_readwrite_encode_write_count_limit(self):
89+
"""Encoding must reject a write count above the FC23 maximum of 121.
90+
91+
121 write registers is a 252 byte PDU, 122 is 254 and does not fit in
92+
the 253 byte MODBUS PDU.
93+
"""
94+
95+
def build(count):
96+
return ReadWriteMultipleRegistersRequest(
97+
read_address=1,
98+
read_count=1,
99+
write_address=1,
100+
write_registers=[0] * count,
101+
)
102+
103+
assert len(build(121).encode()) + 1 <= 253
104+
with pytest.raises(ValueError): # noqa: PT011
105+
build(122).encode()
106+
107+
def test_readwrite_decode_write_count_above_limit(self):
108+
"""Decoding must accept 122..125 so the server can answer ILLEGAL_VALUE.
109+
110+
Raising here would make the server drop the frame without a response.
111+
"""
112+
request = ReadWriteMultipleRegistersRequest()
113+
request.decode(struct.pack(">HHHHB", 1, 1, 1, 122, 244) + b"\x00\x00" * 122)
114+
assert request.write_count == 122
115+
87116
async def test_register_read_requests_count_errors(self, mock_server_context):
88117
"""This tests that the register request messages.
89118

0 commit comments

Comments
 (0)