Skip to content
This repository has been archived by the owner on May 20, 2024. It is now read-only.

Missing XML escaping in GPX export #2203

Closed
wetneb opened this issue Oct 10, 2020 · 3 comments · Fixed by #2204
Closed

Missing XML escaping in GPX export #2203

wetneb opened this issue Oct 10, 2020 · 3 comments · Fixed by #2204

Comments

@wetneb
Copy link
Member

wetneb commented Oct 10, 2020

If a location contains the & character in its description, such as lots of apples & pears, this will translate to the following GPX waypoint:

<wpt lat="1.2345" lon="6.789">
   <name>Lovely place</name>
   <desc>lots of apples & pears</desc>
   <type>places</type>
</wpt>

This is invalid XML since the & character should be escaped: we should have <desc>lots of apples &amp; pears</desc> instead.

This is caused by the jstoxml package used for the export, which does not handle escaping by default (?!). Apparently you can pass a configuration object to specify characters to replace but that's cumbersome. It should escape things by default, as this is a classic vulnerability.

For me that's a red flag - another package should be used instead. I have opened an issue on their side: davidcalhoun/jstoxml#41

@nicksellen
Copy link
Member

Thanks! Yes, I remember being not so content with any of the XML libraries I encountered. Some of the more complete ones were very large, whereas we have very minimal requirements.

One alternative could be to just create the XML as a string... escaping the string content manually. Maybe it's just about 4 lines of code :)

@nicksellen nicksellen mentioned this issue Oct 10, 2020
5 tasks
@wetneb
Copy link
Member Author

wetneb commented Oct 10, 2020

Wow, that was quick!

@nicksellen
Copy link
Member

It looks like it will get fixed soon in davidcalhoun/jstoxml#41, but the test I added should break on the upgrade to let us know we can remove my escaping code :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants