Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sanitize/remove HTML elements if present in the data #1539

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

missinglink
Copy link
Member

@missinglink missinglink commented Jun 24, 2021

As discussed in pelias/model#141 there is a possibility for HTML elements to be returned from the geoJSON API which could have a security impact for unaware clients.

For example, someone could edit OSM and set the name of a place to Foo <script src="evil.js" /> Bar and we would return that verbatim to the user.

Security best-practises on the front-end could mitigate the threat but this could also catch unsuspecting users unaware.

This PR sanitizes/removes HTML elements if present in the data returned from elasticsearch.

There are two options for sanitising, namely 1. completely stripping the element (for <SCRIPT> etc) and 2. removing the elements but keeping the contents (for <P> etc).

I think in an ideal world we could implement this in the importers/model, that would also be cleaner since elasticsearch would never see the HTML in the first place.

However, it's much easier and much more secure to implement it in the pelias/api layer since we can guarantee that the JSON being returned does not contain HTML, whereas we'd have to trust the data indexed in elasticsearch to provide that guarantee if implemented in the importers.

I'm open to either approach, this was just much simpler to implement.

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

Successfully merging this pull request may close these issues.

1 participant