557420 - range.createContextualFragment calls onerror handler in the tagString if it exists

Status: RESOLVED WONTFIX

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

Description

[SAKAI, Kazuaki]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; ja-JP-mac; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; ja-JP-mac; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3

range.createContextualFragment calls img.onerror handler in the tagString if it exists, when reading some specific documents like ow.ly.


Reproducible: Always

Steps to Reproduce:
A reproduciable code is here: http://mozilla:[e-mail]/vulnerablility/01_onerror/
Actual Results:  
called img.onerror handler.

Expected Results:  
range.createContexualFragment must not call img.onerror handlers in the tagString.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #0)
> range.createContexualFragment must not call img.onerror handlers in the
> tagString.

Why not?

Comment 2

[SAKAI, Kazuaki]

1. range.createContextualFragment should not evaluate any scripts because it causes some security risk (see section 2) and it does. 
But only img.oneror and img.onload are evaluated expectionally.

sample code:
> var tagString = <>
>   <script type="text/javascript" src="test.js"></script>
>   <script type="text/javascript"><![CDATA[
>     function foo() { alert("foo"); }
>   ]]></script>
>   <img src="not_found.jpg" onerror="javascript:foo()" />
>   <img src="not_found.jpg" onerror="javascript:alert(1)" />
>   <img src="found.jpg" onload="javascript:bar()" />
> </>.toString();
> 
> var range = document.createRange();
> range.selectNodeContents(document.documentElement);
> range.createContextualFragment(tagString);

test.js:
> function bar() {
>   alert('bar');
> }

Function foo and bar are not evaluated. But onerror and onload handlers are evaluated.
So errors occurred that foo and bar are not defined, and alert(1) is called.


2. This implementation has a potential security risk.
If we want to create a Document from string for the purpose of DOM manipulations,
sometimes we write codes to using document.createDocument and range.createContextualFragment
like this: http://gist.github.com/198443 .
This snipet expected any scripts may not be executed before adding to an actual document.
Safari and Google Chrome do not evaluate on running createContextualFragment.
But Firefox do not act like these.

Comment 3

[Boris Zbarsky [:bzbarsky]]

This looks invalid to me.  If you do this:

  var img = document.createElement("img");
  img.setAttribute("onerror", "alert(1)");
  img.src = "not_found.jpg";

I would expect that Firefox fires the onerror handler.  Wouldn't you?  Certainly Safari/Chrome do so.

The createContextualFragment case is no different.  It creates the exact same DOM, so should have the same behavior.

> This snipet expected any scripts may not be executed before adding to an
> actual document.

This seems like a bogus assumption in general for in-memory DOM trees.  See above createElement example.

> Safari and Google Chrome do not evaluate on running createContextualFragment.

They both show the "XSS" alert for me on your testcase from comment 0, as expected.

Comment 4

[SAKAI, Kazuaki]

> > Safari and Google Chrome do not evaluate on running createContextualFragment.
> 
> They both show the "XSS" alert for me on your testcase from comment 0, as
> expected.

I'm sorry. I had tried to execute sample codes on comment 2, and misunderstood
they do not execute handlers on running range.createContextualFragment.
it is the cause of a syntax error using E4X.

And now I understood createContextualFragment and createElement behave same way.

But I know many addons and userscripts using createContextualFragment without
any validations against input parameter via XMLHttpRequest.
They have potentally vulnerabilities. I think most of them should remove onerror
and onload handlers before using createContextualFragment or use other safe 
ways of getting HTMLDocument from string.

Thanks.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Well, in this case if you had used |doc| instead of |document| to create the range and as the context there would be no script execution, since scripting is disabled in data documents....