Skip to content

Commit

Permalink
Prevent hyperlink handler for potential dangerous URIs
Browse files Browse the repository at this point in the history
This prevents the user from clicking on URIs starting with `javascript` or `data`. The reason behind this is that this may be used to trick users in executing dangerous JS when viewing an untrusted document. (which is the case in our deployment for ownCloud)

I'm not absolutely happy with that patch for multiple reasons, but I consider it a feasible approach:

1. It uses a blacklisting instead of a whitelisting approach. But when it comes to URI schemes there may be a lot of possible values such as `mailto:foo@bar.com` or `ftp://`. That's why I went with this route instead.
2. I originally wanted to check for `javascript:` instead but this fails due to the JSLint policy which then complains about `lib/gui/HyperlinkClickHandler.js:119:28: error: JavaScript URL.`
  • Loading branch information
LukasReschke committed Dec 2, 2014
1 parent 4b06c28 commit 55d71e3
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 2 deletions.
1 change: 1 addition & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ See also section about WebODF

* Fix wrongly enabled hyperlink tools with no document loaded ([#833](https://github.com/kogmbh/WebODF/pull/833))
* Prevent Cross-Site Scripting from style names and font names ([#849](https://github.com/kogmbh/WebODF/pull/849)))
* Prevent Cross-Site Scripting from links ([#850](https://github.com/kogmbh/WebODF/pull/850)))

# Changes between 0.5.3 and 0.5.4

Expand Down
7 changes: 5 additions & 2 deletions webodf/lib/gui/HyperlinkClickHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,11 @@ gui.HyperlinkClickHandler = function HyperlinkClickHandler(getContainer, keyDown
bookmarks[0].scrollIntoView(true);
}
} else {
// Ask the browser to open the link in a new window.
window.open(url);
// Ask the browser to open the link in a new window. `javascript` and `data` URIs are disabled for
// security reasons.
if (url.toLowerCase().indexOf('javascript') !== 0 && url.toLowerCase().indexOf('data') !== 0) {
window.open(url);
}
}

if (e.preventDefault) {
Expand Down

0 comments on commit 55d71e3

Please sign in to comment.