ChargePoint / wireshark-v2g

Dissector for the V2G Protocols
Other
43 stars 18 forks source link

You are using 4 bytes from SDP to get payload type, but payload type is 2 bytes per specification #59

Closed JeremyWhaling closed 4 months ago

JeremyWhaling commented 6 months ago

Hello,

Reviewing the v2g.lua script in further detail, I think I found an issue with your dissect_v2gtp function. You are assigning payload_type_name four bytes from the buffer in this line: _local payload_type_name = get_sdp_payloadtype(buffer(2,4):uint())

Per specification, this is a two byte value, not four. You are essentially relying on the fact that the next two bytes, which are the upper bytes of payload length, would always be zero. In the DIN 70121 and ISO 15118-2 edition 1, this is basically always true. But for ISO 15118-2 edition 2 (SDP with EMSP message), or a malformed SDP message, payload length could exceed two bytes, and ruin the logic for payload_type_name. It seems at some point the types table was extended to 8 bytes to compensate for this, but again if the bytes in payload length are not zero, you'll end up getting a "RESERVED" payload type instead of the expected value. Interestingly, you'll get the correct payload value but not type in the subtree as you are getting the correct buffer here: _subtree:add(SDP["payload_type"], buffer(2,2)):appendtext(

The solution to this I think would be to fix the buffer length from 4 to 2, and update the types table to only two bytes (dropping the two bytes of additional zeros each value has). I can make these changes in a future pull request, unless there is something I'm missing..?

jhart-cpi commented 4 months ago

Should be fixed in https://github.com/ChargePoint/wireshark-v2g/pull/58