Security: purify inputs around innerHTML-usage, remove some unused parameters and classes

This commit is contained in:
Pieter Vander Vennet 2023-09-21 01:53:34 +02:00
parent e0ee3edf71
commit fcea3da70f
15 changed files with 44 additions and 127 deletions

View file

@ -273,7 +273,6 @@ class GenerateSeries extends Script {
allFeatures = allFeatures.filter((f) => f.properties.metadata?.theme !== "EMPTY CS")
const centerpoints = allFeatures.map((f) => GeoOperations.centerpoint(f))
console.log("Found", centerpoints.length, " changesets in total")
const path = `${targetDir}/all_centerpoints.geojson`
const perBbox = GeoOperations.spreadIntoBboxes(centerpoints, options.zoomlevel)

View file

@ -26,7 +26,7 @@ function asList(hist: Map<string, number>): ContributorList {
}
function main() {
exec("git log --pretty='%aN %%!%% %s' ", (error, stdout, stderr) => {
exec("git log --pretty='%aN %%!%% %s' ", (_, stdout) => {
const entries = stdout.split("\n").filter((str) => str !== "")
const codeContributors = new Map<string, number>()
const translationContributors = new Map<string, number>()

View file

@ -34,8 +34,6 @@ function generateTagOverview(
return overview
}
function tagrenderingToTaginfoDescription(tr: TagRenderingConfig) {}
function generateLayerUsage(layer: LayerConfig, layout: LayoutConfig): any[] {
if (layer.name === undefined) {
return [] // Probably a duplicate or irrelevant layer

View file

@ -82,7 +82,7 @@ export default class FeatureSourceMerger implements IndexedFeatureSource {
}
const newList = []
all.forEach((value, key) => {
all.forEach((value) => {
newList.push(value)
})
this.features.setData(newList)

View file

@ -1,7 +1,6 @@
import { OsmNode, OsmObject, OsmWay } from "../../Osm/OsmObject"
import { UIEventSource } from "../../UIEventSource"
import { BBox } from "../../BBox"
import StaticFeatureSource from "../Sources/StaticFeatureSource"
import { Tiles } from "../../../Models/TileRange"
export default class FullNodeDatabaseSource {
@ -48,11 +47,7 @@ export default class FullNodeDatabaseSource {
src.ping()
}
}
const asGeojsonFeatures = Array.from(nodesById.values()).map((osmNode) =>
osmNode.asGeoJson()
)
const featureSource = new StaticFeatureSource(asGeojsonFeatures)
const tileId = Tiles.tile_index(z, x, y)
this.loadedTiles.set(tileId, nodesById)
}

View file

@ -771,7 +771,6 @@ export class GeoOperations {
const splitup = turf.lineSplit(<Feature<LineString>>toSplit, boundary)
const kept = []
for (const f of splitup.features) {
const ls = <Feature<LineString>>f
if (!GeoOperations.inside(GeoOperations.centerpointCoordinates(f), boundary)) {
continue
}

View file

@ -227,8 +227,6 @@ export class OsmConnection {
// details is an XML DOM of user details
let userInfo = details.getElementsByTagName("user")[0]
// let moreDetails = new DOMParser().parseFromString(userInfo.innerHTML, "text/xml");
let data = self.userDetails.data
data.loggedIn = true
console.log("Login completed, userinfo is ", userInfo)

View file

@ -73,7 +73,6 @@ export default class Wikipedia {
if (cached) {
return cached
}
console.log("Constructing store for", cachekey)
const store = new UIEventSource<FullWikipediaDetails>({}, cachekey)
Wikipedia._fullDetailsCache.set(cachekey, store)
@ -123,12 +122,15 @@ export default class Wikipedia {
}
const wikipedia = new Wikipedia({ language: data.language })
wikipedia.GetArticleHtml(data.pagename).then((article) => {
article = Utils.purify(article)
data.fullArticle = article
const content = document.createElement("div")
content.innerHTML = article
const firstParagraph = content.getElementsByTagName("p").item(0)
data.firstParagraph = firstParagraph.innerHTML
content.removeChild(firstParagraph)
if (firstParagraph) {
data.firstParagraph = firstParagraph.innerHTML
content.removeChild(firstParagraph)
}
data.restOfArticle = content.innerHTML
store.ping()
})
@ -194,53 +196,6 @@ export default class Wikipedia {
encodeURIComponent(searchTerm)
return (await Utils.downloadJson(url))["query"]["search"]
}
/**
* Searches via 'index.php' and scrapes the result.
* This gives better results then via the API
* @param searchTerm
*/
public async searchViaIndex(
searchTerm: string
): Promise<{ title: string; snippet: string; url: string }[]> {
const url = `${this.backend}/w/index.php?search=${encodeURIComponent(searchTerm)}&ns0=1`
const result = await Utils.downloadAdvanced(url)
if (result["redirect"]) {
const targetUrl = result["redirect"]
// This is an exact match
return [
{
title: this.extractPageName(targetUrl)?.trim(),
url: targetUrl,
snippet: "",
},
]
}
if (result["error"]) {
throw "Could not download: " + JSON.stringify(result)
}
const el = document.createElement("html")
el.innerHTML = result["content"].replace(/href="\//g, 'href="' + this.backend + "/")
const searchResults = el.getElementsByClassName("mw-search-results")
const individualResults = Array.from(
searchResults[0]?.getElementsByClassName("mw-search-result") ?? []
)
return individualResults.map((result) => {
const toRemove = Array.from(result.getElementsByClassName("searchalttitle"))
for (const toRm of toRemove) {
toRm.parentElement.removeChild(toRm)
}
return {
title: result
.getElementsByClassName("mw-search-result-heading")[0]
.textContent.trim(),
url: result.getElementsByTagName("a")[0].href,
snippet: result.getElementsByClassName("searchresult")[0].textContent,
}
})
}
/**
* Returns the innerHTML for the given article as string.
* Some cleanup is applied to this.
@ -262,7 +217,7 @@ export default class Wikipedia {
if (response?.parse?.text === undefined) {
return undefined
}
const html = response["parse"]["text"]["*"]
const html = Utils.purify(response["parse"]["text"]["*"])
if (html === undefined) {
return undefined
}

View file

@ -1,32 +0,0 @@
import BaseUIElement from "../BaseUIElement"
export class CenterFlexedElement extends BaseUIElement {
private _html: string
constructor(html: string) {
super()
this._html = html ?? ""
}
InnerRender(): string {
return this._html
}
AsMarkdown(): string {
return this._html
}
protected InnerConstructElement(): HTMLElement {
const e = document.createElement("div")
e.innerHTML = this._html
e.style.display = "flex"
e.style.height = "100%"
e.style.width = "100%"
e.style.flexDirection = "column"
e.style.flexWrap = "nowrap"
e.style.alignContent = "center"
e.style.justifyContent = "center"
e.style.alignItems = "center"
return e
}
}

View file

@ -1,5 +1,8 @@
import BaseUIElement from "../BaseUIElement"
import { Utils } from "../../Utils"
/**
* @deprecated
*/
export class FixedUiElement extends BaseUIElement {
public readonly content: string
@ -8,10 +11,6 @@ export class FixedUiElement extends BaseUIElement {
this.content = html ?? ""
}
InnerRender(): string {
return this.content
}
AsMarkdown(): string {
if (this.HasClass("code")) {
if (this.content.indexOf("\n") > 0 || this.HasClass("block")) {
@ -27,7 +26,7 @@ export class FixedUiElement extends BaseUIElement {
protected InnerConstructElement(): HTMLElement {
const e = document.createElement("span")
e.innerHTML = this.content
e.innerHTML = Utils.purify(this.content)
return e
}
}

View file

@ -2,18 +2,14 @@
/**
* Given an HTML string, properly shows this
*/
import DOMPurify from 'dompurify';
import { Utils } from "../../Utils";
export let src: string
let cleaned = DOMPurify.sanitize(src, { USE_PROFILES: { html: true },
ADD_ATTR: ['target'] // Don't remove target='_blank'. Note that Utils.initDomPurify does add a hook which automatically adds 'rel=noopener'
});
let htmlElem: HTMLElement
$: {
if (htmlElem) {
htmlElem.innerHTML = cleaned
htmlElem.innerHTML = Utils.purify(src)
}
}

View file

@ -1,7 +1,11 @@
import { Store } from "../../Logic/UIEventSource"
import BaseUIElement from "../BaseUIElement"
import Combine from "./Combine"
import { Utils } from "../../Utils"
/**
* @deprecated
*/
export class VariableUiElement extends BaseUIElement {
private readonly _contents?: Store<string | BaseUIElement | BaseUIElement[]>
@ -42,7 +46,7 @@ export class VariableUiElement extends BaseUIElement {
return
}
if (typeof contents === "string") {
el.innerHTML = contents
el.innerHTML = Utils.purify(contents)
} else if (contents instanceof Array) {
for (const content of contents) {
const c = content?.ConstructElement()

View file

@ -40,7 +40,7 @@ export default class FediverseValidator extends Validator {
if (match) {
const host = match[2]
try {
const url = new URL("https://" + host)
new URL("https://" + host)
return undefined
} catch (e) {
return Translations.t.validation.fediverse.invalidHost.Subs({ host })

View file

@ -56,7 +56,7 @@ export default class NoteCommentElement extends Combine {
)
const htmlElement = document.createElement("div")
htmlElement.innerHTML = comment.html
htmlElement.innerHTML = Utils.purify(comment.html)
const images = Array.from(htmlElement.getElementsByTagName("a"))
.map((link) => link.href)
.filter((link) => {

View file

@ -25,20 +25,6 @@ Remark that the syntax is slightly different then expected; it uses '$' to note
Note that these values can be prepare with javascript in the theme by using a [calculatedTag](calculatedTags.md#calculating-tags-with-javascript)
`
public static readonly imageExtensions = new Set(["jpg", "png", "svg", "jpeg", ".gif"])
public static initDomPurify() {
if (Utils.runningFromConsole) {
return
}
DOMPurify.addHook("afterSanitizeAttributes", function (node) {
// set all elements owning target to target=_blank + add noopener noreferrer
if ("target" in node) {
node.setAttribute("target", "_blank")
node.setAttribute("rel", "noopener noreferrer")
}
})
}
public static readonly special_visualizations_importRequirementDocs = `#### Importing a dataset into OpenStreetMap: requirements
If you want to import a dataset, make sure that:
@ -160,6 +146,26 @@ In the case that MapComplete is pointed to the testing grounds, the edit will be
}
>()
public static initDomPurify() {
if (Utils.runningFromConsole) {
return
}
DOMPurify.addHook("afterSanitizeAttributes", function (node) {
// set all elements owning target to target=_blank + add noopener noreferrer
if ("target" in node) {
node.setAttribute("target", "_blank")
node.setAttribute("rel", "noopener noreferrer")
}
})
}
public static purify(src: string): string {
return DOMPurify.sanitize(src, {
USE_PROFILES: { html: true },
ADD_ATTR: ["target"], // Don't remove target='_blank'. Note that Utils.initDomPurify does add a hook which automatically adds 'rel=noopener'
})
}
/**
* Parses the arguments for special visualisations
*/