Skip to content

Fix: validate register write byte counts#2977

Merged
janiversen merged 4 commits into
pymodbus-dev:devfrom
MrAlaskan:fix/register-write-byte-count-validation
Jul 23, 2026
Merged

Fix: validate register write byte counts#2977
janiversen merged 4 commits into
pymodbus-dev:devfrom
MrAlaskan:fix/register-write-byte-count-validation

Conversation

@MrAlaskan

Copy link
Copy Markdown
Contributor

Problem

Modbus function codes 0x10 (Write Multiple Registers) and 0x17 (Read/Write Multiple Registers) require the write byte count to equal twice the write quantity, because every register occupies two bytes.

The request decoders did not enforce this relationship. For 0x10, decoding was based on the quantity while the declared byte count was discarded, which could cause extra data to be ignored or truncated data to raise a parsing error. For 0x17, registers were decoded using the byte count, but a byte count inconsistent with the write quantity could still reach the datastore. As a result, malformed requests could be accepted, incorrectly parsed, or fail without returning the required Illegal Data Value (03) response.

Changes

  • Preserve the declared byte count and actual payload length for decoded 0x10 and 0x17 requests, validate both before accessing the datastore, and return Illegal Data Value (03) when they are inconsistent. Parsing now consumes only complete register values so malformed payloads can be rejected cleanly instead of raising struct.error.
  • Add regression tests covering byte counts that are too small, too large, odd, shorter than the actual payload, or followed by extra data. The tests also verify that malformed requests do not write to the datastore and that valid requests continue to succeed.

@janiversen janiversen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please solve formatting problem (ubuntu 3.14).

Please use ./check_ci.sh, that checks and reformats just like CI, and afterward commit changed files.

Comment thread pymodbus/pdu/register_message.py Outdated
register = struct.unpack(">H", data[i : i + 2])[0]
self.write_registers.append(register)
self._payload_byte_count = len(data) - 9
payload = data[9 : self.write_byte_count + 9]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why make an extra split ? Line 154 could just use data directly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. The registers are now unpacked directly from data. I applied the same simplification to both 0x17 and 0x10.

@janiversen janiversen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

@janiversen
janiversen merged commit 8a3be74 into pymodbus-dev:dev Jul 23, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants