Last modified: 2014-06-30 20:16:39 UTC
I'm running MediaWiki 1.23alpha. Having the following test code in one of my extensions: Html::element( 'a', array( 'href' => "javascript:window.alert('danger!')" ), 'Click here' ); ...displays a link that, when clicked on, pops up an alert. I'm told that this is not correct behavior, so I'm submitting a bug for it.
(In reply to Yaron Koren from comment #0) I'm told that > this is not correct behavior, so I'm submitting a bug for it. By whom? While it might not be the most fortunate behavior, Html::element only HTML-escapes the attributes and does not mangle their contents. You could validate user input by checking it against the list of protocols returned by wfUrlProtocols(), or using Sanitizer::validateTagAttributes() to do more thorough cleanup of other attributes as well.
We discussed it in the comments here: https://gerrit.wikimedia.org/r/#/c/124995/ But based on what you're saying, it sounds like there was just a misunderstanding about escaping vs. mangling of Javascript content.
Changing to product "Security" - should have done that at the beginning; sorry.
(In reply to Yaron Koren from comment #2) > We discussed it in the comments here: > > https://gerrit.wikimedia.org/r/#/c/124995/ > > But based on what you're saying, it sounds like there was just a > misunderstanding about escaping vs. mangling of Javascript content. That patches doesn't do anything to resolve any HTML injection issue, it just incorrectly double-escapes user input (which does make it more difficult to do XSS by preventing you from using quotes, angle brackets and ampersands, but by no means impossible). This code looks like it should be using Sanitizer::validateTagAttributes() instead.
Bartosz - alright, I'll try that out. Back to the main issue: I should note that, as far as I can remember, calling Html::element() by itself was enough to disable the Javascript, in some earlier version of MediaWiki. The relevant call is in my "HTML Tags" extension, and I was surprised recently to see that it wasn't mangling the Javascript, where I believe the same code was doing that before, with an earlier version (MW 1.19, maybe). It could be just faulty memory, though.
Html::element outputs whatever you tell it to. If you are sanitizing user input, that's your job to do it -- that's why we have a big ol' Sanitizer class etc for stuff that comes in from wikitext.
Html::element will sanitize attributes against attacks that add new attributes to the Html element, or attacks that try to add new html elements inside that element's body. But if a user is able to influence the entire attribute for hrefs (or iframes, or styles, or onmouseover...), or influence what attributes are added in the first place, then that's a whole separate set of concerns. For hrefs specifically, you would want to either build the link with the Linker class, or use one of the other Sanitizer functions to limit what kinds of urls you allow to be added. As Bartosz suggested, you probably want to use validateTagAttributes on anything that users have supplied. It has code specifically to handle hrefs for cases like this.
Is there a reason why this report is still private?
