Skip to content

Commit ef44d0f

Browse files
brhergerloi-opfeljaniversen
authored
ensure unpacking of proper length during decoding (#2664) (#2665)
Co-authored-by: Beau Herger <beau.herger@msasafety.com> Co-authored-by: jan iversen <jancasacondor@gmail.com>
1 parent b4cecc3 commit ef44d0f

File tree

7 files changed

+103
-37
lines changed

7 files changed

+103
-37
lines changed

‎pymodbus/pdu/bit_message.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ def encode(self) -> bytes:
2323

2424
def decode(self, data: bytes) -> None:
2525
"""Decode a request pdu."""
26-
self.address, self.count = struct.unpack(">HH", data)
26+
self.address, self.count = struct.unpack(">HH", data[:4])
2727

2828
def get_response_pdu_size(self) -> int:
2929
"""Get response pdu size.
@@ -84,7 +84,7 @@ def encode(self) -> bytes:
8484

8585
def decode(self, data: bytes) -> None:
8686
"""Decode a write coil request."""
87-
self.address, value = struct.unpack(">HH", data)
87+
self.address, value = struct.unpack(">HH", data[:4])
8888
self.bits = [bool(value)]
8989

9090

@@ -157,4 +157,4 @@ def encode(self) -> bytes:
157157

158158
def decode(self, data: bytes) -> None:
159159
"""Decode a write coils response."""
160-
self.address, self.count = struct.unpack(">HH", data)
160+
self.address, self.count = struct.unpack(">HH", data[:4])

‎pymodbus/pdu/diag_message.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,9 @@ def decode(self, data: bytes) -> None:
5454
data_len += 1
5555
data += b"0"
5656
if (word_len := data_len // 2) == 1:
57-
(self.message,) = struct.unpack(">H", data)
57+
(self.message,) = struct.unpack(">H", data[:2])
5858
else:
59-
self.message = struct.unpack(">" + "H" * word_len, data)
59+
self.message = struct.unpack(">" + "H" * word_len, data[:2 * word_len])
6060

6161
def get_response_pdu_size(self) -> int:
6262
"""Get response pdu size.

‎pymodbus/pdu/file_message.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ def encode(self) -> bytes:
235235

236236
def decode(self, data: bytes) -> None:
237237
"""Decode the incoming request."""
238-
self.address = struct.unpack(">H", data)[0]
238+
self.address = struct.unpack(">H", data[:2])[0]
239239

240240
async def update_datastore(self, _context: ModbusDeviceContext) -> ModbusPDU:
241241
"""Run a read exception status request against the store."""

‎pymodbus/pdu/mei_message.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ def encode(self) -> bytes:
4949

5050
def decode(self, data: bytes) -> None:
5151
"""Decode data part of the message."""
52-
self.sub_function_code, self.read_code, self.object_id = struct.unpack(">BBB", data)
52+
self.sub_function_code, self.read_code, self.object_id = struct.unpack(">BBB", data[:3])
5353

5454
async def update_datastore(self, _context: ModbusDeviceContext) -> ModbusPDU:
5555
"""Run a read exception status request against the store."""

‎pymodbus/pdu/other_message.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ def encode(self) -> bytes:
8181

8282
def decode(self, data: bytes) -> None:
8383
"""Decode a the response."""
84-
ready, self.count = struct.unpack(">HH", data)
84+
ready, self.count = struct.unpack(">HH", data[:4])
8585
self.status = ready == ModbusStatus.READY
8686

8787

‎pymodbus/pdu/register_message.py‎

Lines changed: 87 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
"""Register Reading Request/Response."""
2+
23
from __future__ import annotations
34

45
import struct
@@ -24,7 +25,7 @@ def encode(self) -> bytes:
2425

2526
def decode(self, data: bytes) -> None:
2627
"""Decode a register request packet."""
27-
self.address, self.count = struct.unpack(">HH", data)
28+
self.address, self.count = struct.unpack(">HH", data[:4])
2829

2930
def get_response_pdu_size(self) -> int:
3031
"""Get response pdu size.
@@ -38,8 +39,16 @@ async def update_datastore(self, context: ModbusDeviceContext) -> ModbusPDU:
3839
values = await context.async_getValues(
3940
self.function_code, self.address, self.count
4041
)
41-
response_class = (ReadHoldingRegistersResponse if self.function_code == 3 else ReadInputRegistersResponse)
42-
return response_class(registers=cast(list[int], values), dev_id=self.dev_id, transaction_id=self.transaction_id)
42+
response_class = (
43+
ReadHoldingRegistersResponse
44+
if self.function_code == 3
45+
else ReadInputRegistersResponse
46+
)
47+
return response_class(
48+
registers=cast(list[int], values),
49+
dev_id=self.dev_id,
50+
transaction_id=self.transaction_id,
51+
)
4352

4453

4554
class ReadHoldingRegistersResponse(ModbusPDU):
@@ -59,7 +68,9 @@ def decode(self, data: bytes) -> None:
5968
"""Decode a register response packet."""
6069
self.registers = []
6170
if (data_len := int(data[0])) >= len(data):
62-
raise ModbusIOException(f"byte_count {data_len} > length of packet {len(data)}")
71+
raise ModbusIOException(
72+
f"byte_count {data_len} > length of packet {len(data)}"
73+
)
6374
for i in range(1, data_len, 2):
6475
self.registers.append(struct.unpack(">H", data[i : i + 2])[0])
6576

@@ -82,13 +93,15 @@ class ReadWriteMultipleRegistersRequest(ModbusPDU):
8293
function_code = 23
8394
rtu_byte_count_pos = 10
8495

85-
def __init__(self,
86-
read_address: int = 0x00,
87-
read_count: int = 0,
88-
write_address: int = 0x00,
89-
write_registers: list[int] | None = None,
90-
dev_id: int = 1,
91-
transaction_id: int = 0) -> None:
96+
def __init__(
97+
self,
98+
read_address: int = 0x00,
99+
read_count: int = 0,
100+
write_address: int = 0x00,
101+
write_registers: list[int] | None = None,
102+
dev_id: int = 1,
103+
transaction_id: int = 0,
104+
) -> None:
92105
"""Initialize a new request message."""
93106
if not write_registers:
94107
write_registers = []
@@ -135,9 +148,13 @@ def decode(self, data: bytes) -> None:
135148
async def update_datastore(self, context: ModbusDeviceContext) -> ModbusPDU:
136149
"""Run a write single register request against a datastore."""
137150
if not (1 <= self.read_count <= 0x07D):
138-
return ExceptionResponse(self.function_code, ExceptionResponse.ILLEGAL_VALUE)
151+
return ExceptionResponse(
152+
self.function_code, ExceptionResponse.ILLEGAL_VALUE
153+
)
139154
if not 1 <= self.write_count <= 0x079:
140-
return ExceptionResponse(self.function_code, ExceptionResponse.ILLEGAL_VALUE)
155+
return ExceptionResponse(
156+
self.function_code, ExceptionResponse.ILLEGAL_VALUE
157+
)
141158
rc = await context.async_setValues(
142159
self.function_code, self.write_address, self.write_registers
143160
)
@@ -146,7 +163,11 @@ async def update_datastore(self, context: ModbusDeviceContext) -> ModbusPDU:
146163
registers = await context.async_getValues(
147164
self.function_code, self.read_address, self.read_count
148165
)
149-
return ReadWriteMultipleRegistersResponse(registers=cast(list[int], registers), dev_id=self.dev_id, transaction_id=self.transaction_id)
166+
return ReadWriteMultipleRegistersResponse(
167+
registers=cast(list[int], registers),
168+
dev_id=self.dev_id,
169+
transaction_id=self.transaction_id,
170+
)
150171

151172
def get_response_pdu_size(self) -> int:
152173
"""Get response pdu size.
@@ -174,7 +195,7 @@ def encode(self) -> bytes:
174195

175196
def decode(self, data: bytes) -> None:
176197
"""Decode a write single register packet packet request."""
177-
self.address, register = struct.unpack(">HH", data)
198+
self.address, register = struct.unpack(">HH", data[:4])
178199
self.registers = [register]
179200

180201

@@ -184,14 +205,18 @@ class WriteSingleRegisterRequest(WriteSingleRegisterResponse):
184205
async def update_datastore(self, context: ModbusDeviceContext) -> ModbusPDU:
185206
"""Run a write single register request against a datastore."""
186207
if not 0 <= self.registers[0] <= 0xFFFF:
187-
return ExceptionResponse(self.function_code, ExceptionResponse.ILLEGAL_VALUE)
208+
return ExceptionResponse(
209+
self.function_code, ExceptionResponse.ILLEGAL_VALUE
210+
)
188211
rc = await context.async_setValues(
189212
self.function_code, self.address, self.registers
190213
)
191214
if rc:
192215
return ExceptionResponse(self.function_code, rc)
193216
values = await context.async_getValues(self.function_code, self.address, 1)
194-
return WriteSingleRegisterResponse(address=self.address, registers=cast(list[int], values))
217+
return WriteSingleRegisterResponse(
218+
address=self.address, registers=cast(list[int], values)
219+
)
195220

196221
def get_response_pdu_size(self) -> int:
197222
"""Get response pdu size.
@@ -225,13 +250,20 @@ def decode(self, data: bytes) -> None:
225250
async def update_datastore(self, context: ModbusDeviceContext) -> ModbusPDU:
226251
"""Run a write single register request against a datastore."""
227252
if not 1 <= self.count <= 0x07B:
228-
return ExceptionResponse(self.function_code, ExceptionResponse.ILLEGAL_VALUE)
253+
return ExceptionResponse(
254+
self.function_code, ExceptionResponse.ILLEGAL_VALUE
255+
)
229256
rc = await context.async_setValues(
230257
self.function_code, self.address, self.registers
231-
)
258+
)
232259
if rc:
233260
return ExceptionResponse(self.function_code, rc)
234-
return WriteMultipleRegistersResponse(address=self.address, count=self.count, dev_id=self.dev_id, transaction_id=self.transaction_id)
261+
return WriteMultipleRegistersResponse(
262+
address=self.address,
263+
count=self.count,
264+
dev_id=self.dev_id,
265+
transaction_id=self.transaction_id,
266+
)
235267

236268
def get_response_pdu_size(self) -> int:
237269
"""Get response pdu size.
@@ -253,7 +285,7 @@ def encode(self) -> bytes:
253285

254286
def decode(self, data: bytes) -> None:
255287
"""Decode a write single register packet packet request."""
256-
self.address, self.count = struct.unpack(">HH", data)
288+
self.address, self.count = struct.unpack(">HH", data[:4])
257289

258290

259291
class MaskWriteRegisterRequest(ModbusPDU):
@@ -262,7 +294,14 @@ class MaskWriteRegisterRequest(ModbusPDU):
262294
function_code = 0x16
263295
rtu_frame_size = 10
264296

265-
def __init__(self, address=0x0000, and_mask=0xFFFF, or_mask=0x0000, dev_id=1, transaction_id=0) -> None:
297+
def __init__(
298+
self,
299+
address=0x0000,
300+
and_mask=0xFFFF,
301+
or_mask=0x0000,
302+
dev_id=1,
303+
transaction_id=0,
304+
) -> None:
266305
"""Initialize a new instance."""
267306
super().__init__(transaction_id=transaction_id, dev_id=dev_id, address=address)
268307
self.and_mask = and_mask
@@ -274,22 +313,34 @@ def encode(self) -> bytes:
274313

275314
def decode(self, data: bytes) -> None:
276315
"""Decode the incoming request."""
277-
self.address, self.and_mask, self.or_mask = struct.unpack(">HHH", data)
316+
self.address, self.and_mask, self.or_mask = struct.unpack(">HHH", data[:6])
278317

279318
async def update_datastore(self, context: ModbusDeviceContext) -> ModbusPDU:
280319
"""Run a mask write register request against the store."""
281320
if not 0x0000 <= self.and_mask <= 0xFFFF:
282-
return ExceptionResponse(self.function_code, ExceptionResponse.ILLEGAL_VALUE)
321+
return ExceptionResponse(
322+
self.function_code, ExceptionResponse.ILLEGAL_VALUE
323+
)
283324
if not 0x0000 <= self.or_mask <= 0xFFFF:
284-
return ExceptionResponse(self.function_code, ExceptionResponse.ILLEGAL_VALUE)
325+
return ExceptionResponse(
326+
self.function_code, ExceptionResponse.ILLEGAL_VALUE
327+
)
285328
values = await context.async_getValues(self.function_code, self.address, 1)
286-
values = (cast(Sequence[int | bool], values)[0] & self.and_mask) | (self.or_mask & ~self.and_mask)
329+
values = (cast(Sequence[int | bool], values)[0] & self.and_mask) | (
330+
self.or_mask & ~self.and_mask
331+
)
287332
rc = await context.async_setValues(
288333
self.function_code, self.address, cast(list[int], [values])
289334
)
290335
if rc:
291336
return ExceptionResponse(self.function_code, rc)
292-
return MaskWriteRegisterResponse(address=self.address, and_mask=self.and_mask, or_mask=self.or_mask, dev_id=self.dev_id, transaction_id=self.transaction_id)
337+
return MaskWriteRegisterResponse(
338+
address=self.address,
339+
and_mask=self.and_mask,
340+
or_mask=self.or_mask,
341+
dev_id=self.dev_id,
342+
transaction_id=self.transaction_id,
343+
)
293344

294345

295346
class MaskWriteRegisterResponse(ModbusPDU):
@@ -298,7 +349,14 @@ class MaskWriteRegisterResponse(ModbusPDU):
298349
function_code = 0x16
299350
rtu_frame_size = 10
300351

301-
def __init__(self, address=0x0000, and_mask=0xFFFF, or_mask=0x0000, dev_id=1, transaction_id=0) -> None:
352+
def __init__(
353+
self,
354+
address=0x0000,
355+
and_mask=0xFFFF,
356+
or_mask=0x0000,
357+
dev_id=1,
358+
transaction_id=0,
359+
) -> None:
302360
"""Initialize new instance."""
303361
super().__init__(transaction_id=transaction_id, dev_id=dev_id, address=address)
304362
self.and_mask = and_mask
@@ -311,4 +369,4 @@ def encode(self) -> bytes:
311369

312370
def decode(self, data: bytes) -> None:
313371
"""Decode a the response."""
314-
self.address, self.and_mask, self.or_mask = struct.unpack(">HHH", data)
372+
self.address, self.and_mask, self.or_mask = struct.unpack(">HHH", data[:6])

‎test/framer/test_framer.py‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,14 @@ def test_roundtrip_CRC(self):
6969
assert FramerRTU.compute_CRC(data) == 0xE2DB
7070
assert FramerRTU.check_CRC(data, 0xE2DB)
7171

72+
async def test_handleFrame2(self):
73+
"""Test handleFrame."""
74+
test_framer = FramerRTU(DecodePDU(True))
75+
msg = b"\xfe\x04\x00\x03\x00\x01\xd5\xc5\x00"
76+
used_len, pdu = test_framer.handleFrame(msg, 0, 0)
77+
assert used_len == len(msg)
78+
assert pdu
79+
7280

7381
class TestFramerType:
7482
"""Test classes."""

0 commit comments

Comments
 (0)