Last Comment Bug 368655 - [FIX]Easy DoS by <img src="javascript:for(;;);"> even if javascript disabled
: [FIX]Easy DoS by <img src="javascript:for(;;);"> even if javascript disabled
Status: RESOLVED FIXED
[sg:low dos]
: regression, verified1.8.0.10, verified1.8.1.2
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- critical (vote)
: mozilla1.9alpha1
Assigned To: Boris Zbarsky [:bz]
: Hixie (not reading bugmail)
Mentors:
data:text/html,<img src="javascript:f...
: 362049 (view as bug list)
Depends on:
Blocks: 351370
  Show dependency treegraph
 
Reported: 2007-01-29 20:38 PST by Anbo Motohiko
Modified: 2007-07-31 08:08 PDT (History)
7 users (show)
dveditz: blocking1.8.1.2+
dveditz: blocking1.8.0.10+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix (1.92 KB, patch)
2007-01-29 21:23 PST, Boris Zbarsky [:bz]
jst: review+
jst: superreview+
Details | Diff | Splinter Review
Branch patch (2.53 KB, patch)
2007-01-31 17:07 PST, Boris Zbarsky [:bz]
jst: review+
dveditz: approval1.8.1.2+
dveditz: approval1.8.0.10+
Details | Diff | Splinter Review

Description Anbo Motohiko 2007-01-29 20:38:35 PST
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 [:bz] 2007-01-29 21:21:06 PST
Oooh, yeah.  I bet this is a regression from bug 351370.  Patch coming up in a sec, I hope.
Comment 2 Boris Zbarsky [:bz] 2007-01-29 21:23:25 PST
Created attachment 253284 [details] [diff] [review]
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, jst@mozilla.com) 2007-01-29 22:17:47 PST
Comment on attachment 253284 [details] [diff] [review]
Fix

This seems good to me. r+sr=jst
Comment 4 Boris Zbarsky [:bz] 2007-01-30 08:39:13 PST
Fixed on trunk.
Comment 5 Boris Zbarsky [:bz] 2007-01-30 08:39:42 PST
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 [:bz] 2007-01-30 09:54:05 PST
I filed bug 368714 on the lack of a slow script dialog.
Comment 7 Daniel Veditz [:dveditz] 2007-01-30 15:28:20 PST
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] 2007-01-31 14:31:49 PST
Comment on attachment 253284 [details] [diff] [review]
Fix

Clearing flags as this patch relies on trunk-only components.
Comment 9 Boris Zbarsky [:bz] 2007-01-31 16:52:41 PST
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 [:bz] 2007-01-31 17:07:59 PST
Created attachment 253559 [details] [diff] [review]
Branch patch

This works for the branches.
Comment 11 Johnny Stenback (:jst, jst@mozilla.com) 2007-01-31 17:34:23 PST
Comment on attachment 253559 [details] [diff] [review]
Branch patch

r=jst
Comment 12 Daniel Veditz [:dveditz] 2007-02-01 13:36:12 PST
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 [:bz] 2007-02-01 18:57:02 PST
Fixed on branches.
Comment 14 Anbo Motohiko 2007-02-02 22:11:45 PST
(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 [:bz] 2007-02-02 22:17:52 PST
> 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] 2007-02-06 16:03:05 PST
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 [:bz] 2007-02-06 16:07:38 PST
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] 2007-02-06 16:17:37 PST
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] 2007-03-02 20:51:29 PST
Thunderbird was not affected by this
Comment 20 :Gavin Sharp [email: gavin@gavinsharp.com] 2007-07-31 08:08:36 PDT
*** Bug 362049 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.