Skip to content

modbus: count the run indicator in Report Slave ID byteCount - #5249

Open
bIackr0se wants to merge 1 commit into
secdev:masterfrom
bIackr0se:fix/modbus-report-slave-id-byte-count
Open

bIackr0se wants to merge 1 commit into
secdev:masterfrom
bIackr0se:fix/modbus-report-slave-id-byte-count

Conversation

@bIackr0se

Copy link
Copy Markdown

Description

In a Report Slave ID response, byteCount counts the slave ID and the run indicator status byte (the Modbus application protocol specification, function 0x11; pymodbus and libmodbus encode it this way). ModbusPDU11ReportSlaveIdResponse uses byteCount as the slave ID length. As a result, building the response without an explicit byteCount raises TypeError, and a response from a device does not dissect.

from scapy.contrib.modbus import *
ModbusADUResponse(bytes.fromhex("00010000000fff110c41636d652d504c432d3031ff"))  # reply of a pymodbus 3.16.1 server
bytes(ModbusPDU11ReportSlaveIdResponse(slaveId=b"PLC-01", runIdicatorStatus=0xff))

Before:

<ModbusADUResponse  transId=0x1 protoId=0x0 len=15 unitId=0xff |<Raw  load=b'\x11\x0cAcme-PLC-01\xff' |>>
TypeError: While building field 'slaveId': '>' not supported between instances of 'NoneType' and 'int'

After:

<ModbusADUResponse  transId=0x1 protoId=0x0 len=15 unitId=0xff |<ModbusPDU11ReportSlaveIdResponse  funcCode=0x11 byteCount=12 slaveId=b'Acme-PLC-01' runIdicatorStatus=0xff |>>
b'\x11\x07PLC-01\xff'

The existing test raw(ModbusPDU11ReportSlaveIdResponse(byteCount=3, slaveId="ID")) == b'\x11\x03\x49\x44\x00' already uses this meaning of byteCount and still passes.

This message was written with the help of AI (Claude Opus 5.5).

byteCount of a Report Slave ID response covers the slave ID and the
run indicator status byte. The layer used it as the slave ID length,
so a response could not be built without an explicit byteCount, and a
response from a device was dissected as Raw.

AI-Assisted: yes (Claude Opus 5.5, GPT-6.1 Sol)
@codecov

codecov Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.18%. Comparing base (bf1b540) to head (8e7743a).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5249      +/-   ##
==========================================
+ Coverage   81.17%   81.18%   +0.01%     
==========================================
  Files         394      394              
  Lines       98770    98770              
==========================================
+ Hits        80178    80188      +10     
+ Misses      18592    18582      -10     
Files with missing lines Coverage Δ
scapy/contrib/modbus.py 92.17% <ø> (ø)

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant