From 9cc87a1911e3994ebf92903140da902560016754 Mon Sep 17 00:00:00 2001 From: Colin S <3526918+cbs228@users.noreply.github.com> Date: Tue, 6 Sep 2022 22:16:50 -0500 Subject: [PATCH] Make inserted marker text secure-by-default Per the documentation, Overviewer configuration authors must take special precautions to escape user-generated HTML and/or JavaScript from marker signs. In mide/minecraft-overviewer#117 [1], we learned that: * This detail may escape the notice of downstream packagers and server deployments. * It is very straightforward to inject JavaScript into signs on Creative servers. It may also be possible on Survival servers. Overviewer should conform to MDN's recommendations for safely inserting external content [2]. Failure to protect against injected scripts may allow a malicious actor to gain control of other web resources on the same DNS name (i.e., via session hijacking). In all likelihood, of course, people will just make nuisances of themselves. A small change in the way text is handled in JavaScript can eliminate the need to invoke `html.escape()` at all in python. The Leaflet `bindPopup()` method can accept a DOM node instead of a text string [3]. The leaflet method inserts text strings via an *unsafe* `innerHTML` assignment. DOM nodes are inserted verbatim [4]. This patch creates a text-only DOM node that contains the marker text. Text nodes are not subject to further interpretation, and all HTML characters appear as literals. We use CSS to permit newline characters to render as line breaks. This preserves multi-line marker text. References 1. https://github.com/mide/minecraft-overviewer/issues/117 2. https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Safely_inserting_external_content_into_a_page 3. https://leafletjs.com/reference.html#layer-bindpopup 4. https://github.com/Leaflet/Leaflet/blob/f10b44b3afcd079febe5219f84dc67c68b379b5e/src/layer/DivOverlay.js#L280 --- overviewer_core/data/js_src/util.js | 4 +++- overviewer_core/data/web_assets/leaflet.css | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/overviewer_core/data/js_src/util.js b/overviewer_core/data/js_src/util.js index f633eda47..4f9bda4b9 100644 --- a/overviewer_core/data/js_src/util.js +++ b/overviewer_core/data/js_src/util.js @@ -416,7 +416,9 @@ overviewer.util = { } // Add popup to marker if (marker_entry.createInfoWindow && db.text) { - layerObj.bindPopup(db.text); + layerObj.bindPopup( + document.createTextNode(db.text) + ); } // Add the polyline or marker to the layer marker_group.addLayer(layerObj); diff --git a/overviewer_core/data/web_assets/leaflet.css b/overviewer_core/data/web_assets/leaflet.css index 601476fe6..c757d3c79 100644 --- a/overviewer_core/data/web_assets/leaflet.css +++ b/overviewer_core/data/web_assets/leaflet.css @@ -472,6 +472,7 @@ svg.leaflet-image-layer.leaflet-interactive path { padding: 1px; text-align: left; border-radius: 12px; + white-space: pre-line; } .leaflet-popup-content { margin: 13px 19px;