HTML encoding for KML: option to use numeric character entities instead of entity references? about owasp-java-encoder

[jmanico]

Hello! Thanks for the thoughtful post. We use < because it saves a few bytes in the output. One of the primary goals of this lib is performance and •minimal• encoding. I politely vote no, we do not want to support non-browser edge cases like this. But I do suggest you fork the library for your needs. This project is very stable and has had very few changes in the last years. Forking and maintaining a version of the project should not be a hassle. Happy to continue discussion if you like. Aloha, -- Jim Manico @manicode Secure Coding Education [phone]

…

On Jun 19, 2018, at 11:59 PM, ╭∩╮ (òÓ,) ╭∩╮ ***@***.***> wrote: First of all, please forgive me posting it here. This is rather a shortcoming in the KML specification or the Google Earth implementation, respectively. But I am not familiar with Google's issue tracking system and the following could be a useful enhancement for owasp-java-encoder, too. What's the problem? I just realized that KML files I am generating are vulnerable to injection despite (in my opinion correct) use of HTML encoding powered by owasp-java-encoder. Owasp's Encode.forHtml/forXml works correctly: the input <strike>oops</strike> gets encoded to &lt;strike&gt;oops&lt;/strike&gt;. I do not want my users to inject HTML. However, the KML specification is unclear about encoding of angle brackets, please see KML Reference Errata. And the software consuming the KML (Google Earth) will render it as HTML. How can this problem be solved? According to the errata page mentioned above, numeric character entities have to be used in favor of entity references, i.e. < and > must be encoded to &#60; and &#62; instead of &lt;and &gt;. From looking at owasp's HTMLEncoder/XMLEncoder classes I cannot find an option to control the encode behavior. Do you think this would be a reasonable enhancement? Alternatively, do you think introducing a special KMLEncoder would be reasonable? How to reproduce? Download Google Earth, start it and then load a KML file with this content: <?xml version="1.0" encoding="UTF-8" standalone="yes"?> <kml xmlns="http://www.opengis.net/kml/2.2" xmlns:gx="http://www.google.com/kml/ext/2.2"> <Document> <name><![CDATA[HTML encoding]]></name> <Style id="myStyle"> <BalloonStyle> <text> <![CDATA[ Text should be bold only: <b>$[text]</b> ]]> </text> </BalloonStyle> </Style> <Folder> <name>Placemarks</name> <Placemark> <name><![CDATA[insufficient escaping]]></name> <address><![CDATA[1200-C Agora Drive, Bel Air, MD 21014, US]]></address> <styleUrl><![CDATA[#myStyle]]></styleUrl> <ExtendedData> <Data name="text"> <value><![CDATA[&lt;strike&gt;oops&lt;/strike&gt;]]></value> </Data> </ExtendedData> </Placemark> <Placemark> <name><![CDATA[sufficient escaping]]></name> <address><![CDATA[Leinstraat 104A, B-9660 Opbrakel, Belgium]]></address> <styleUrl><![CDATA[#myStyle]]></styleUrl> <ExtendedData> <Data name="text"> <value><![CDATA[&#60;strike&#62;fine&#60;/strike&#62;]]></value> </Data> </ExtendedData> </Placemark> </Folder> </Document> </kml> Open the placemark called insufficient escaping and see the strikethrough word: Open the placemark called sufficient escaping and see everything's working correctly: Please note that this issue can not be reproduced when uploading the KML file to the web-based Google Earth. Things may have changed in the online version. However, the desktop version is still maintained (last build: February 6, 2018). — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[cnsgithub]

Hello and thanks for your honest answer. I fully understand your point of not adding support for edge cases and keeping it small and simple. I don't know why Google Earth decided to implement it this way...

As I already have a delegator class in place to avoid dependencies to org.owasp.encoder throughout my code I worked around this issue by just adding this one:

	/**
	 * This is a special case of XML encoding that replaces angle brackets with numeric character entities (< and >) instead of entity references (< and >).
	 * This prevents Google Earth from rendering inadvertent HTML.
	 * @see <a href="http://kml4earth.appspot.com/kmlErrata.html?#encoding">KML Reference Errata</a>
	 * @see <a href="https://github.com/OWASP/owasp-java-encoder/issues/20">OWASP Issue</a>
	 */
	public static String kmlEncode(String str) {
		return org.owasp.encoder.Encode.forXml(str).replace("&lt;", "&#60;").replace("&gt;", "&#62;");
	}

I know that strictly replacing all entity references by their numerical representations would be the better but more complex approach. For the moment it solves the problem and I didn't manage to break out and inject HTML. Do you think the workaround sufficiently prevents all cases? Let me see, maybe forking your repo should be preferred in the end...

Bye

[jmanico]

This looks quite reasonable to me. It's a directly entity to entity replacement without complex logic. I like this solution! Aloha, Jim

…

On 6/20/18 10:36 PM, ╭∩╮ (òÓ,) ╭∩╮ wrote: Hello and thanks for your honest answer. I fully understand your point of not adding support for edge cases and keeping it small and simple. I don't know why Google Earth decided to implement it this way... As I already have a delegator class in place to avoid dependencies to |org.owasp.encoder| throughout my code I worked around this issue by just adding this one: /** * This is a special case of XML encoding that replaces angle brackets with numeric character entities (&amp;#60; and &amp;#62;) instead of entity references (&amp;lt; and &amp;gt;). * This prevents Google Earth from rendering inadvertent HTML. * @see <a href="http://kml4earth.appspot.com/kmlErrata.html?#encoding">KML Reference Errata</a> * @see <a href="#20">OWASP Issue</a> */ public static String kmlEncode(String str) { return org.owasp.encoder.Encode.forXml(str).replace("&lt;", "&#60;").replace("&gt;", "&#62;"); } I know that strictly replacing all entity references by their numerical representations would be the better but more complex approach. For the moment it solves the problem and I didn't manage to break out and inject HTML. Do you think the workaround sufficiently prevents all cases? Let me see, maybe forking your repo should be preferred in the end... Bye — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#20 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAgcCUYtPuNxp3Wh_SCHzqeG0uZmlfXSks5t-yKzgaJpZM4Uutdn>.

[cnsgithub]

I also integrated a specialized KMLEncoder in https://github.com/cnsgithub/owasp-java-encoder. We can safely close this issue.