Closed Bug 217195 Opened 17 years ago Closed 17 years ago
security hole in mark
Link Visited (exploit with link .href setter = eval)
Impact: full compromise. Interaction required: The user must open a link from the exploit page in a new window or new tab. Vulnerable code: // from markLinkVisited in contentAreaUtils.js var oldHref = linkNode.href; linkNode.href = ""; linkNode.href = oldHref; Exploit: Set a link node's |href setter| to the built-in function |eval|. This makes the third line pasted from markLinkVisited call eval with oldHref as the parameter. See also bug 187195, that line should be linkNode.setAttribute("href", oldHref).
When you try the demo, the second alert, which says "[object nsXPCComponents_Classes]", shows that the script has chrome privs.
* Fixes this security hole. * Fixes a minor security hole where the attacker makes chrome convert linkNode.href to a string for you (similar to bug 202994). * Fixes bug 187195.
who would be good reviewers? let get this one in soon..
Comment on attachment 130506 [details] [diff] [review] patch Rather than do this, let's land the patch for bug 169000 and do this the right way. I had been waiting to land that until something came up which could use it, and this definitely does. We can use the API I wrote up there to get the real getter method here. The problem with this patch his that setAttribute and getAttribute can also be over-ridden...
Need a spot-fix for 1.5 final -- shouldn't we go with this bug's patch? /be
brendan: from caillon's comment, it sounds like this spot fix is not a valid fix. the problem just gets shifted.
> The problem with this patch his that setAttribute and > getAttribute can also be over-ridden... I don't think that's exploitable, at least not in the same way. If a page replaces setAttribute with eval, chrome evals the string "href", which evaluates to a string without doing any real computation.
Also, my patch would be correct even without this security bug. See bug 187195, "when unvisited link becomes visited, href attribute becomes absolute".
Brendan, this is a firebird-only bug. Is there an imminent firebird release we need this for? The real fix IMO would be to get rid of the weird hack we're doing with setting the href to "" and then back to its original value just to get it to pick up a visited state. What we really need is a "setAsVisited()" or similar method on somewhere which does the right thing here.
Yes, Firebird 0.7 is coming up fast, coincident with Mozilla 1.5. Cleaning up the hack with a method sounds good, but how will the method be implemented? Probably in XBL, via the hack ;-). /be
I was thinking of something in c++, actually. Possibly nsILink or nsILinkHandler? They are both currently unscriptable, though...
Is there anything that needs to happen other than to set the link state and force a repaint?
Fixed, branch and trunk, 0.7
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
I think we need a general solution whereby native methods such as eval cannot be set as thunks to be called from chrome, to run arbitrary input with chrome privs. I've opened bug 223041 on this problem. /be
Was there a reason not to just use an XPCWrapper here? That would avoid the issues with pages overriding getAttribute/setAttribute too....
You need to log in before you can comment on or make changes to this bug.