132657 - International chars in <IMG SRC=...> leads to crash

Status: RESOLVED DUPLICATE of bug 138877

(Core :: Security, defect)

Description

[Mitchell Stoltz (not reading bugmail)]

There is a problem with <img src="...international chars...">.
Some memory gets badly overwritten, may be exploitable.
Yesterday's nightly crashed in an exploitable way - it ended in
user-specified memory.
Today's nightly crashed in free() which is more difficult to exploit.
Seems like someone has committed changes.
How to reproduce:
imgchars.pl is a perl cgi.
Access it, expected result is crash.
If it does not crash, click on the submit button (points to non-existent
url) the click 'back' from the toolbar.

Comment 1

[Mitchell Stoltz (not reading bugmail)]

Attachment: imgchars.pl - access this as a cgi to see the exploit

Comment 2

[georgi - hopefully not receiving bugspam]

This is similar to bug 128825 http://bugzilla.mozilla.org/show_bug.cgi?id=128825
which is claimed to be fixed but this bug still remains.
I am almost sure this is exploitable the only difficulty being the return
address must be unicode.

Comment 3

[Asa Dotzler [:asa]]

Mitch, we're pushing RC1 today (builds 2002041711). Are we likely to get this in
time for RC2 (about 2 weeks) or 1.0 final (about 3 or 4 weeks, hopefully)?

Comment 4

[Mitchell Stoltz (not reading bugmail)]

dougt has agreed to take this (thanks Doug!) 

Here's what I've found so far: The crash appears to be heap corruption, since
the call stack at crash time varies on each run but is always in a malloc or
free operation. For the crash to occur, the first character of the path portion
of the IMG SRC url must have a value greater than 128, and it must be followed
by at least 15-20 more chars of any value.

Comment 5

[Mitchell Stoltz (not reading bugmail)]

Over to dougt

Comment 6

[Mitchell Stoltz (not reading bugmail)]

Attachment: Simpler testcase - run this perl file as a cgi.

Here's a simpler testase. If you're inside Netscape's firewall, it's also at
http://grey/cgi-bin/bo2b.pl

Comment 7

[Doug Turner (:dougt)]

Basically, we are trusting the content type of the page as the correct key into
our url segment parsing code, which of course is wrong.

The fix is clean and small.

Comment 8

[Doug Turner (:dougt)]

Attachment: Enforce charset on new uri's

Since the charset of the spec is known, we hard code it.  I am not sure if I
should being a NS_LITERAL_CSTRING?

This does not optimize callers to do the right thing.  For example the source
of the problem could remove the charset parameter and pass null.  In fact every
caller of NS_NewURI using a astring could ignore the charset flag:

http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsImageFrame.cpp#2030


and I am somewhat worried about this code (did it every work?): 

http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDefaultURIFixup.cpp#121



Darin can you verify that this is the correct thing to do, or give an example
why we would pass a UTF-8 spec in and lie about the charset?

Comment 9

[Doug Turner (:dougt)]

darin says this is a dup which was fixed 

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