modbus icon indicating copy to clipboard operation
modbus copied to clipboard

TCP length header included device ID?

Open muhlemmer opened this issue 4 years ago • 0 comments

I wanted to learn more about the MODBUS TCP protocol and found this library and started reading the sources to get a better understanding. I came across this in the tcpTransporter.Send() method:

https://github.com/goburrow/modbus/blob/606c02f4eef527a1d4cf7d8733d5fd7ba34f91d8/tcpclient.go#L173-L178

Where:

https://github.com/goburrow/modbus/blob/606c02f4eef527a1d4cf7d8733d5fd7ba34f91d8/tcpclient.go#L22-L23

Unless I'm not seeing it right: After io.ReadFull, data has 7 bytes populated, remaining 253 bytes are 0x00. Decoding it with data[4:] practically passes 3 populated bytes.

This will result in length to contain a larger value if the device ID is non zero. This could then lead to a false positive on the length error check further down the code.

Also, the aduResponse might end up being longer. This wouldn't be a problem if the PDU unpacking takes proper precautions on lengths, haven't checked.

https://github.com/goburrow/modbus/blob/606c02f4eef527a1d4cf7d8733d5fd7ba34f91d8/tcpclient.go#L194

I'm not actually using this project, so I'm not able to confirm if this an actual bug and I might be completely wrong about my assumption. Just trying to learn new things :D.

muhlemmer avatar May 16 '20 15:05 muhlemmer