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:
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.
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.
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,datahas 7 bytes populated, remaining 253 bytes are0x00. Decoding it withdata[4:]practically passes 3 populated bytes.This will result in
lengthto contain a larger value if the device ID is non zero. This could then lead to a false positive on thelengtherror check further down the code.Also, the
aduResponsemight 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.