Skip to content

Fix: validate byte count in file_message.py decode handlers - #2980

Merged
janiversen merged 6 commits into
pymodbus-dev:devfrom
SunandM:fix/file-message-byte-count-validation
Jul 24, 2026
Merged

Fix: validate byte count in file_message.py decode handlers#2980
janiversen merged 6 commits into
pymodbus-dev:devfrom
SunandM:fix/file-message-byte-count-validation

Conversation

@SunandM

@SunandM SunandM commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Adds bounds checking to three decode handlers in pymodbus/pdu/file_message.py,
consistent with the fixes applied in #2974 and #2977.

Changes:

  • ReadFileRecordRequest.decode: validate byte_count against len(data) before iterating
  • WriteFileRecordRequest.decode: validate byte_count against len(data) before looping
  • ReadFifoQueueResponse.decode: validate count against len(data) before iterating

Previously, an oversized byte_count field caused struct.unpack() to raise
struct.error, which escaped the ModbusException/ValueError/IndexError
handler in DecodePDU.decode(). The fix raises ModbusException instead,
matching the behaviour added in #2974 and #2977.

Related to security advisory GHSA-xxxx (private).

SunandM and others added 3 commits July 24, 2026 08:20
Adds bounds checking to ReadFileRecordRequest, WriteFileRecordRequest,
and ReadFifoQueueResponse decode handlers, consistent with pymodbus-dev#2974 and pymodbus-dev#2977.

Raises ModbusException instead of struct.error when byte_count exceeds
actual payload length.

@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, but you need to add tests:

Captura de pantalla 2026-07-24 a las 15 35 10

The new "if" statements are not covered in the test harness and we are proud to have 100% coverage.

easiest way to test locally:

run "purest --cov" in the main directory.

@SunandM

SunandM commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

Hi @janiversen, I have added three tests covering the new validation branches (test_read_file_record_request_decode_invalid_byte_count, test_write_file_record_request_decode_invalid_byte_count, test_read_fifo_queue_response_decode_invalid_count). All 16 CI checks are now passing with 100% coverage. Could you take another look when you get a chance? Thanks!

@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 09a3cb2 into pymodbus-dev:dev Jul 24, 2026
16 checks passed
@SunandM

SunandM commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

Thank you so much for your assistance :)

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