201839 - Clicking link during paint suppression loads link as if it were part of paint-suppressed page

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Jesse Ruderman]

This bug affects referers and cross-domain security, and might also affect
session history.

Steps to reproduce:
1. Load http://www.cs.hmc.edu/~jruderma/s/ .
2. Click the "Slashdot" link.
3. As soon as the title bar changes (but before the Slashdot page appears),
click the "mpt" link.

Result: Slashdot appears briefly, then mpt appears. (MIGHT NOT BE THIS BUG)
Expected: Only mpt should appear. (MIGHT NOT BE THIS BUG; related to bug 78681)

4. Back.

Result: Slashdot appears. (MIGHT NOT BE THIS BUG)
Expected: www.cs.hmc.edu/~jruderma/s/ should appear. (MIGHT NOT BE THIS BUG)

5. Forward.
6. Open Page Info, look at referer.

Result: Referer for mpt.phrasewise.com is slashdot.org.
Expected: Referer for mpt.phrasewise.com should be www.cs.hmc.edu/~jruderma/s/.

7. Try out the security testcases (which involve javascript: URLs).

Result: Cross-domain security hole.
Expected: No security hole.

Comment 1

[Jesse Ruderman]

Attachment: security testcase 1: click when onunload changes background color

This one only works in Mozilla builds from the last few months.

Comment 2

[Jesse Ruderman]

Attachment: security testcase 2: click when titlebar changes

This one works in older Mozilla builds, too.

Comment 3

[Jesse Ruderman]

Attachment: security testcase 3: click as rapidly as you can

This testcase is the best for turning the exploit into a game (cf bug 162020),
but it isn't as good for seeing what's going on as the other testcases.

Comment 4

[Jesse Ruderman]

Possible fixes include:
A. Disallow clicking links while a new page is paint-suppressed.
B. Make links load with the correct referer (if http or https), context (if
javascript: or data:), etc. even if a new page is paint-suppressed.

Comment 5

[Bradley Baetz (:bbaetz)]

I'd vote for (b), based on least-surpise, most obvious result, etc

Comment 6

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Isn't this a duplicate of bug 201132? There is a patch there that we should
check in today (first version had to be backed out earlier).

Comment 7

[Johnny Stenback (:jst)]

Turns out that this is not a dupe of bug 201132, this is indeed a different problem.

Comment 8

[Johnny Stenback (:jst)]

The issue here is specific to javascript: URL's, the problem is that once the
page where one of these links is has been unloaded, yet still displayed,
clicking on a link to a javascript: URL will execute in the new document that is
now being loaded into the window.

I think we should simply don't execute javascript: URL's coming from a 'zombie'
document, if we'd do that, we'd plug this hole... Doing that should be fairly
straight forward, I'll see what I can do tomorrow unless someone else wants to
take a stab at this.

Comment 9

[Jesse Ruderman]

jst: That won't fix the referer problem.  But if you're going to do that as a
quick fix, you'll probably need to block data: URLs from zombie pages too.

Comment 10

[Johnny Stenback (:jst)]

Attachment: Don't let links from zombie documents execute JS.

This plugs this hole in the link handler in Mozilla. In theory, we could
support plugging in different link handlers into Mozilla, and if we ever
support that, the new link handlers need to be aware of this. I added a comment
in nsWebShell::GetInterface() where a change would be needed if we were ever to
support that.

Comment 11

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Comment on attachment 120990 [details] [diff] [review]
Don't let links from zombie documents execute JS.

sr=heikki

I think the referrer issue is different enough to be solved in a separate bug.

Comment 12

[Johnny Stenback (:jst)]

Yeah, that only fixes the cross-domain security exploit, it doesn't fix the
referer problem.

I had a look at the referer problem too, and it should be easy to fix. One more
patch coming up.

Comment 13

[Johnny Stenback (:jst)]

Attachment: Don't let links from zombie documents execute JS. And use the link's document's URI as the referer.

This fixes both the security problem, and the problem with the referer.

Comment 14

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Comment on attachment 121013 [details] [diff] [review]
Don't let links from zombie documents execute JS. And use the link's document's URI as the referer.

I think it should be ok to load a link with empty referrer URL.

Comment 15

[Mitchell Stoltz (not reading bugmail)]

Comment on attachment 121013 [details] [diff] [review]
Don't let links from zombie documents execute JS. And use the link's document's URI as the referer.

Looks good. r=mstoltz.

Comment 16

[Mitchell Stoltz (not reading bugmail)]

Reassigning to jst. Thanks for the quick patch, Johnny!

Comment 17

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Comment on attachment 121013 [details] [diff] [review]
Don't let links from zombie documents execute JS. And use the link's document's URI as the referer.

So if passing empty referrer works (no crashes, assertions, grabbing the wrong
referrer later somewhere else) sr=heikki

Comment 18

[Johnny Stenback (:jst)]

Yeah, passing no referer works, so I took out the if check. Fix checked in.

FIXED.