226495 - Mozilla parses "half-tags" gullibly, leading to XSS security problems

Status: RESOLVED WONTFIX

(Core :: DOM: HTML Parser, defect)

Description

[Seth Woolley]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6a) Gecko/20031103 Firebird/0.7+
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6a) Gecko/20031103 Firebird/0.7+

Mozilla interprets invalid half-tags as if the author meant that they were full
tags, leading to possible security cookie theft through javascript injection
into, for example, comment forms.  Early versions of MovableType, for example,
suffered from this combination vulnerability, for rather than converting angle
brackets into entity references, it stripped out html tags, leaving half a tag
behind to cause havoc with the mozilla HTML Parser.

Mozilla should not interpret invalid markup in a way that favors the ability to
inject executable code.  If assuming invalid code as "html" in general and not
"content" is favorable in most cases, this "feature" should be disabled for
script tags and any tags that contain script-interpreting attributes.  If a
half-tag contains a script-interpreting attribute, perhaps the parser should
close the tag before that attribute instead of waiting until the next opening
angle bracket.

As an alternative, one could merely take all javasript attributes and attributes
that refer to an url (src, href, etc) and close the tag there instead of waiting
until the next opening angle bracket.  Then one doesn't have to treat script
tags differently.

Reproducible: Always

Steps to Reproduce:
1.leave off the right angle bracket and end tag from a script open tag when
posting text.
2.post this text to a server script that filters out full tags "<.*?>" instead
of converting brackets to entity references before displaying content back to
the user (common mistake).

1. or just go the url http://smgl.positivism.org/music/indexvuln.html to see an
example.

Actual Results:  
XSS vulnerability pathway happens in Mozilla but not other browsers because it
makes the incomplete tag turn into a full tag, with closing tag, too.

'></script>' is added after the attributes and before the next '<'.

Expected Results:  
It should have converted the '<' to '&lt;' instead of making up '></script>' to
put where it thought it went.

With this bug, I feel that browsing in mozilla and using cookies is rather
unsafe on sites with public comments that also have a cookie authentication.  I
consider security to be a major feature of mozilla, and it is quite broken if
such interactions can be created through a server-side programming accident that
normally wouldn't seem as bad as one would think, and I have seen it in the wild.

Comment 1

[Seth Woolley]

will reproduce on a newer build ... rebuilding Firebird now...

Comment 2

[Jesse Ruderman]

I tested Firebird, IE, Opera, and Netscape 4 on that page.  Firebird is the only
one that shows bold text.  Opera spins and never finishes loading the page,
probably because the SCRIPT tag is never closed.

The corresponding view-source problem is bug 70918.

Comment 3

[Jesse Ruderman]

*** This bug has been marked as a duplicate of 213383 ***

Comment 4

[Stewart Gordon]

No bug in Mozilla.  Bug in the form validation/correction code.

Comment 5

[Seth Woolley]

It's not the same bug.  I agree with your interpretation of the other bugs, but
the problem exists in _where_ you interpret the '>', not _whether_ you interpret
the '>'.  So far there has been no discussion on where you interpret the '>'.

If you fix it by inserting the '>' before the first ' ', you'll end up with
perfectly well-nested code for further parsing, but you won't be interpreting
the attributes, downloading code, and executing it.  Of course, I suggested
above you insert '>' before / (href|src|on[a-z]+)=|</i instead of just / / or
/</, to be more lenient to web developers.

I don't challenge that this bug only manifests in improper code
translation/validation situations, either, but, since it is common that this
mistake is made, I think it's necessary to be pre-emptively defensive and take
the position that errors such as this mean that it's not trusted to execute
arbitrary code.

BTW, I did rebuild a newer version and duplicated it on a Nov 22 CVS HEAD build.

Comment 6

[Jesse Ruderman]

Reopening based on comment 5.  I don't understand comment 5, but I guess a
parser person should look at it.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Actually, as I understand it, proper parsing of SGML/HTML _requires_ that we
behave this way.  That is, the '<' of the next tag is a valid way to close out
the markup of a previous tag...

choess, is that the case?

Comment 8

[Jesse Ruderman]

Full-disclosure thread about this bug:
http://lists.netsys.com/pipermail/full-disclosure/2004-July/023942.html

Comment 9

[Boris Zbarsky [:bzbarsky]]

Note http://lists.netsys.com/pipermail/full-disclosure/2004-July/023995.html

Also note that changing this behavior is likely to be a very difficult and
dangerous HTMLParser change; there _are_ cases when IE will do SHORTTAG properly
here (what exact cases?  I don't know; that's why it's dangerous -- it's very
easy to break a lot of markup that's out there.  But I recall bugs on it).   So
just breaking this HTML feature (_required_ by the spec) altogether is not
really feasible.

'm not sure anyone who understands the parser well enough to do some sort of
evil hack for just the script case can actually be found...

Comment 10

[Hixie (not reading bugmail)]

Actually if you study this you'll find that both IE and Opera do the same as 
Mozilla, that is, they parse the <script> tag and put it into the DOM and so 
forth. I examined IE in particular, and found that it even fetches the JS file 
pointed to by the src attribute. What I don't understand is why IE then ignores 
that file -- it doesn't seem to do anything with it.

Comment 11

[Hixie (not reading bugmail)]

Aha. The trick is to give IE a "</script" sequence, that's when it executes the 
script. So this "security problem" can be reproduced in IE:

   http://www.hixie.ch/tests/adhoc/html/parsing/compat/019.html

I think this is INVALID. Just like the second full-disclosure post above says, 
the pages that are susceptible to this are faulty. They should be allowing safe 
stuff through, not blocking unsafe stuff. (There are literally dozens of ways of 
executing script in IE.)

Comment 12

[Boris Zbarsky [:bzbarsky]]

Note that IE generally won't execute the script in the following markup, last I
checked (at least not consistently; I could never figure out the pattern):

<head>
<script src="whatever" type="javascript">
</head>

I had to add </script> all over to my DOM tests to get it to run the scripts...

Comment 13

[Hixie (not reading bugmail)]

The pattern is that it will execute the script as soon as it sees the "</script" 
tag. (Note the lack of ">", which is relevant to this bug, and is why I think we 
should call this invalid.)

Comment 14

[Sander Goudswaard]

See also 
http://www.zapthedingbat.com/security/scriptinjection/

Comment 15

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #14)
> See also 
> http://www.zapthedingbat.com/security/scriptinjection/

that's totally unrelated

Comment 16

[Blake Kaplan (:mrbkap) (inactive)]

This is going to be fixed by bug 305873. I'm marking this as WONTFIX since the summary seems to suggest that we shouldn't do this parsing for other tags, which we won't change.

Note that IE actually requires a > (so the document: |<script>alert("hi")</script| doesn't execute a script).