scapy
scapy copied to clipboard
secdev
Scapy.contrib.modbus cannot handle packets with multiple ADU/PDU sets
Brief description
In non-spec packets where multiple ADU/PDU layers exist in parallel the first Modbus ADU Response layer consumes the entire payload. This causes registerVal fields in ModbusPDU03 and 04 responses (at least), to end up significantly larger than expected and missing other ModbusTCPResponse packets.
Scapy version
2.5.0
Python version
3.11.3
Operating system
Windows 10
Additional environment information
No response
How to reproduce
from scapy.all import *
import scapy.contrib.modbus as mb
packets = PcapReader("FailedPackets.pcap")
for pkt in packets:
if (mb.ModbusPDU04ReadInputRegistersResponse in pkt):
#print (str(inc) +":" + str(pkt[mb.ModbusPDU04ReadInputRegistersResponse].fields))
pdureglen = len(pkt[mb.ModbusPDU04ReadInputRegistersResponse].fields["registerVal"])
pduregval = (pkt[mb.ModbusPDU04ReadInputRegistersResponse].fields["byteCount"])
#if (pdureglen != (pduregval/2)):
print (pdureglen,int(pduregval/2),sep=":")
Actual result
132:99 Observe that we have 132 registerVal words, when we expected 99 based on bytecount
Expected result
99:99
Related resources
I'm working on fixing this myself, but I'm not familiar with Scapy. It appears we need two steps to a solution. First step is properly calculating the length of each packet. in the PDU, that's as easy as assigning the length from the packet data itself to the length_from.
FieldListField("registerVal", [0x0000],
ShortField("", 0x0000),
length_from=lambda pkt: pkt.byteCount,
count_from=lambda pkt: pkt.byteCount)]
In the ADU, well, I haven't figured out how to interpret the len variable from the packet during dissection without breaking the build function.
The second step is to implement extract_padding. I'll know if I get there.
Step 1: Properly calculate the length of the PDU using: length_from=lambda pkt: pkt.byteCount
Step 2: Extract PDU padding with
def extract_padding(self, s):
pay = s[:self.byteCount]
pad = s[self.byteCount:]
return pay, pad
Step 3: Extract ADU Padding with
def extract_padding(self, s):
pay = s[:self.len-1]
pad = s[self.len-1:]
return pay, pad
Step 4: Add a layer that contains a PacketListField
name = "ModbusTCPResponse"
fields_desc = [
PacketListField("ADU_List",None,ModbusADUResponse)
]
Step 5: Bind the new layer like: bind_layers(TCP, ModbusTCPResponse, sport=502)
Result:
\ADU_List \
|###[ ModbusADU ]###
| transId = 0x7cfe
| protoId = 0x0
| len = 201
| unitId = 0xff
|###[ Read Input Registers Response ]###
| funcCode = 0x4
| byteCount = 198
| registerVal= [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 4, 0, 0, 0, 0, 0, 0, 0, 475, 0, 470, 0, 19000, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 25697, 26989, 110, 0, 0, 0, 0, 0, 0, 0, 12337, 48, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
|###[ ModbusADU ]###
| transId = 0x7cff
| protoId = 0x0
| len = 7
| unitId = 0xff
|###[ Read Input Registers Response ]###
| funcCode = 0x4
| byteCount = 4
| registerVal= [4, 0]
|###[ ModbusADU ]###
| transId = 0x7d00
| protoId = 0x0
| len = 47
| unitId = 0xff
|###[ Read Input Registers Response ]###
| funcCode = 0x4
| byteCount = 44
| registerVal= [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
This works, but I'm not convinced it's still the best way. I suspect there is a way to not use the PacketListField and still have each ADU be a sibling not a child.
Are there unit tests associated with this module?
I have run and passed the unit tests for this module. I intend to expand them in lieu of further guidance. I have not contributed to an open source project. What is my next step here, now that I have a potential solution to present?
