Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Include magnet scheme in sanitize HTML params #1301

Merged
merged 1 commit into from
Oct 14, 2017
Merged

Include magnet scheme in sanitize HTML params #1301

merged 1 commit into from
Oct 14, 2017

Conversation

zeroware
Copy link
Contributor

Update HtmlUtils sanitze-html params to include the magnet scheme

Update HtmlUtils sanitze-html params to include the magnet scheme
@ara4n
Copy link
Member

ara4n commented Sep 17, 2017

@dbkr: what do you think about this? I'm a bit worried about allowing more exotic schemes in case there are vulns in how they're implemented in some platforms (e.g. XSS attacks due to the URI handler managing to execute JS embedded in the URL or whatever).

@zeroware: for this to merge we'd also need it whitelisted in ios-sdk and android-sdk too for consistency. But hold off first until we get feedback on the above q.

@dbkr
Copy link
Member

dbkr commented Sep 20, 2017

It's probably OK I think. It's only a concern if the browser manages to execute js in the URL in the course of parsing it or it being clicked on, and we shouldn't be doing any per-protocol parsing. If the thing that's handling the URL manages to execute arbitrary JS, it would have the same problem if that link were clicked from a website / email.

@dbkr dbkr assigned ara4n and unassigned dbkr Sep 20, 2017
@ara4n
Copy link
Member

ara4n commented Oct 14, 2017

ok. lgtm then.

@ara4n ara4n merged commit 67ba0e5 into matrix-org:develop Oct 14, 2017
@zeroware zeroware deleted the develop branch October 15, 2017 09:37
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants