924252 - javascript in <img src="javascript:..."> is executed on the main thread (should be in a worker)

Status: RESOLVED INVALID

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

Description

[yan]

Attachment: ffxss.html

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:24.0) Gecko/20100101 Firefox/24.0 (Beta/Release)
Build ID: 20130911160237

Steps to reproduce:

1. Insert the following into any HTML page:
<img src="javascript:var i; while(true){i++;}">
2. View the page in Firefox (tested with FF24 on Linux, FF25 on OSX)

Equivalently, you can load the attached HTML file into Firefox.



Actual results:

The page freezes and gives an alert that says: "A script on this page may be busy, or it may have stopped responding . . ."


Expected results:

Nothing should have happened, since Firefox deprecated support for the "javascript:" external protocol in image sources in v2.0, according to http://stackoverflow.com/questions/3303886/what-browsers-allow-javascript-to-be-the-source-of-image-tags.

Note that <img src="javascript:alert(1);"> simply gives an ReferenceError (alert is not defined). It seems that the javascript can't access browser-defined variables, but this doesn't stop us from doing DOS attacks.

Comment 1

[Benjamin Smedberg]

It doesn't appear that this is a security issue in itself, just a DOS. But before I open this up we need to know that running JS via image handlers isn't exploitable in other ways.

Comment 2

[Boris Zbarsky [:bzbarsky]]

<img src="javascript:"> runs the script in a sandbox, not against the page.  It has no access to any of the page's objects.

Given that this is not the only way to DoS with an arbitrary image URL (e.g. you could use a small data: png that expands to a multi-hundred-gigabyte decompressed image), I'm not sure it's worth worrying about the DoS angle of this overmuch either...

What's more interesting is that our current behavior is content-detectable: you can actually output image data that will render.  The question is whether the spec will support that or not; if not, we should consider removing this.

Comment 3

[Benjamin Smedberg]

What's the point of a javascript: URI that *couldn't* output image data? Isn't that the whole point?

If the script runs in a sandbox, could we just run it in a worker? That way it would load asynchronously and we could apply a timeout.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> If the script runs in a sandbox, could we just run it in a worker?

Hmm.  I believe we could, yes.

Comment 5

[yan]

Could you use sandboxed JS to create heap memory corruption? In any case, Chrome doesn't have this bug.

Comment 6

[Boris Zbarsky [:bzbarsky]]

> Could you use sandboxed JS to create heap memory corruption?

If there are bugs in the JS engine.

> In any case, Chrome doesn't have this bug.

Chrome's support for javascript: URIs is very restricted.  They don't handle them as URIs at all, in fact, which leads to all sorts of spec noncompliance in Chrome.

Comment 7

[Benjamin Smedberg]

I'm going to open this up; it's just a DOS, which we don't typically keep hidden. I'm also going to morph the summary to mention moving this evaluation into a worker, since that would solve the DOS.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Bobby, would it be viable to create XPConnect sandbox objects on a non-main thread?

I assume that we do in fact want to run this in a sandbox, not a full-up Worker, because those expose networking APIs like XHR that we don't want to expose here.

Comment 10

[Bobby Holley (:bholley)]

(In reply to Not doing reviews right now from comment #9)
> Bobby, would it be viable to create XPConnect sandbox objects on a non-main
> thread?
> 
> I assume that we do in fact want to run this in a sandbox, not a full-up
> Worker, because those expose networking APIs like XHR that we don't want to
> expose here.

That would involve security wrappers on workers, which is doable, but not something we'd do lightly.

It seems like a quicker path to victory would just be to define a worker context with no interfaces exposed and nothing exposed by the bindings on the global. Then the worker itself is a sandbox. This might be useful for other things too.

Comment 11

[Jinank Jain[:jj]]

I would like to work on this could someone help me where to start of it :)

Comment 12

[Boris Zbarsky [:bzbarsky]]

Given bug 1018583 and its fix, I think this bug is now just invalid.  We don't do the sandbox thing at all anymore.