368655 - [FIX]Easy DoS by <img src="javascript:for(;;);"> even if javascript disabled

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Anbo Motohiko]

User-Agent:       Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.8.1.2pre) Gecko/20070128 BonEcho/2.0.0.2pre
Build Identifier: Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.8.1.2pre) Gecko/20070128 BonEcho/2.0.0.2pre

Firefox allows javascript scheme in src attribute of img element.
Current branch nightlies can nothing because of Bug 344890 (there are no window object), but the script runs even if I disable JavaScript.
(sorry, I can't test with trunk)

And if there is an infinite loop in this script, the "script runs slowly" dialog does not shown.


Tested and occurs on Firefox 2.0.0.2pre/20070128/WinMe and 1.5.0.10pre/20070129/WinMe.

Reproducible: Always

Steps to Reproduce:
1.Try URL above.
Actual Results:  
Firefox hangs.

Expected Results:  
At least, the "script runs slowly" dialog appears.

I want to ask Mozilla folks: why do you allow javascript scheme in src attribute of img element? According to Bug 179793 comment #0, this feature is for the compatibility with Netscape4 and Windows IE 6. Opera 9.10 allow this (but Opera doesn't hangs whole applicaiton), but I've heard of that Windows IE7 doesn't allow this. I think this /feature/ is not needed at all.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Oooh, yeah.  I bet this is a regression from bug 351370.  Patch coming up in a sec, I hope.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

The other option is to put that check in the sandbox code, but I'm not sure we want that...

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 253284 [details] [diff] [review]
Fix

This seems good to me. r+sr=jst

Comment 4

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 253284 [details] [diff] [review]
Fix

This makes sure to not execute javascript: URIs if script is disabled for the page, even if we were going to execute them in a sandbox.

Comment 6

[Boris Zbarsky [:bzbarsky]]

I filed bug 368714 on the lack of a slow script dialog.

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 253284 [details] [diff] [review]
Fix

We'll need a different patch for the branches, right? No nullprincipal back then.

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 253284 [details] [diff] [review]
Fix

Clearing flags as this patch relies on trunk-only components.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Sorry about the lag here.  I was kinda busy for a bit, and the answer for branches is not exactly trivial.

So all that this method wants out of a principal is to check whether it's chrome, and if not to check whether its URI is in a whitelist (about:, about:neterror, that sort of thing).

So I think it should be safe enough to use the about:blank principal.  I'll comment accordingly in the patch coming up.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Branch patch

This works for the branches.

Comment 11

[Johnny Stenback (:jst)]

Comment on attachment 253559 [details] [diff] [review]
Branch patch

r=jst

Comment 12

[Daniel Veditz [:dveditz]]

Comment on attachment 253559 [details] [diff] [review]
Branch patch

approved for 1.8/1.8.0 branches, a=dveditz for drivers

Comment 13

[Boris Zbarsky [:bzbarsky]]

Fixed on branches.

Comment 14

[Anbo Motohiko]

(In reply to comment #9)
> Sorry about the lag here.  I was kinda busy for a bit, and the answer for
> branches is not exactly trivial.
> 
> So all that this method wants out of a principal is to check whether it's
> chrome, and if not to check whether its URI is in a whitelist (about:,
> about:neterror, that sort of thing).
> 
> So I think it should be safe enough to use the about:blank principal.  I'll
> comment accordingly in the patch coming up.
> 

On 2.0.0.2pre 2007020204, when I open URI above, the "script runs slowly"
dialog appears (thank you for fixing Bug 368714).

When I disable JS on a page by using CAPS, the script still runs. Is this intended? I made a simple test for this.
http://www1.ttcn.ne.jp/amotohiko/temp/bug368655.html

Comment 15

[Boris Zbarsky [:bzbarsky]]

> When I disable JS on a page by using CAPS

You mean the "javascript" policy for the page?

It's somewhat "intended" in that we don't really know who started the load.  So we don't know which CAPS policy is relevant.  I suppose we could check whether script is enabled in general (for the about:blank principal) and if it is also check for the principal of the target context.  I'm not sure whether that gets us very far (e.g. I think you can work around that by getting an image from a subframe, then loading a different page in the subframe, so the target context is no longer from your domain), and I'm not sure how commonly the CAPS thing is even used (unlike the general "disable javascript" option, which does work now).

The real way to fix that is to track principals for all javascript: loads and not execute the script at all if we don't have an origin principal.  That's what I'd like to aim to do for 1.9...

Comment 16

[juan becerra [:juanb]]

Verified on branches and trunk. 1509 and 2001 would hang on the url on top. Now after a few seconds you get: "A script on this page may be busy, or it may have stopped responding. You can stop the script now, or you can continue to see if the script will complete", and you get the option to Stop or Continue.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2pre) Gecko/20070206 BonEcho/2.0.0.2pre
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.10pre) Gecko/20070206 Firefox/1.5.0.10pre

Also tested on Mac, Linux 1.8.1.2 builds.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Juan, that's the verification for bug 368714, not the patch that landed in this bug.  To verify this bug you need to disable JavaScript in your browser and then verify that this script doesn't execute at all....  Sorry about the way this got split up into two bugs.  :(

Comment 18

[juan becerra [:juanb]]

I should have noted in comment #16 that I also tried disabling Javascript in Options/Content. The script was not executed after disabling Javascript.

Comment 19

[Daniel Veditz [:dveditz]]

Thunderbird was not affected by this