ChargePoint / wireshark-v2g

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

XML Entity � silently dropped from ProtocolNamespace strings #44

Closed malsyned closed 1 year ago

malsyned commented 1 year ago

An EVCC I encountered during compatibility testing transmitted a supportedAppProtocolReq whose ProtocolNamespace URNs all ended with �. This is firstly an invalid XML entity, and then if a lenient parser translates it, it's still a null byte:

<?xml version="1.0" encoding="UTF-8"?>
<ns4:supportedAppProtocolReq xmlns:ns4="urn:iso:15118:2:2010:AppProtocol"
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:ns3="http://www.w3.org/2001/XMLSchema">
  <AppProtocol>
    <ProtocolNamespace>urn:din:70121:2012:MsgDef&#0;</ProtocolNamespace>
    <VersionNumberMajor>2</VersionNumberMajor>
    <VersionNumberMinor>0</VersionNumberMinor>
    <SchemaID>10</SchemaID>
    <Priority>15</Priority>
  </AppProtocol>
  <AppProtocol>
    <ProtocolNamespace>urn:iso:15118:2:2013:MsgDef&#0;</ProtocolNamespace>
    <VersionNumberMajor>2</VersionNumberMajor>
    <VersionNumberMinor>0</VersionNumberMinor>
    <SchemaID>20</SchemaID>
    <Priority>20</Priority>
  </AppProtocol>
  <AppProtocol>
    <ProtocolNamespace>urn:iso:15118:2:2015:MsgDef&#0;</ProtocolNamespace>
    <VersionNumberMajor>3</VersionNumberMajor>
    <VersionNumberMinor>0</VersionNumberMinor>
    <SchemaID>30</SchemaID>
    <Priority>10</Priority>
  </AppProtocol>
</ns4:supportedAppProtocolReq>

However, wireshark-v2g displayed the ProtocolNamespace strings without any warning, error, or mention of the null character at the end of them: image

Expected result: either some kind of error message about the invalid XML entity, or at least a '\0' or something at the end of the strings to indicate that they're unusual, like [ProtocolNamespace: urn:din:70121:2012:MsgDef\0]

Actual result: [ProtocolNamespace: urn:din:70121:2012:MsgDef]

Example pcap: null-in-protocolnamespace-strings.zip

malsyned commented 1 year ago

It looks to me like the null byte is getting dropped in v2gexi.h:exi_add_characters(), where a UCS string is converted into a C string before adding it to the tree. A null character will just cause the string to terminate early, and any character code >= 256 will have its high bits stripped off of it.

Wireshark's documentation has some guidance for how they'd like strings like these displayed at https://wiki.wireshark.org/Development/String-handling-in-dissectors#displayed-to-users but they don't mention any API calls that could be used to make that easier. The Wireshark function epan/dfilter/sttype-pointer.c:charconst_tostr() looks like it gives a helpful template, though.

malsyned commented 1 year ago

Sorry for all the spam on this ticket, I didn't realize that the work I was doing in my own repo would be reflected here.

jhart-cpi commented 1 year ago

Sorry for all the spam on this ticket, I didn't realize that the work I was doing in my own repo would be reflected here.

No problem, thank you for fixing this issue.