255120 - crash if a textbox/textarea has a value of 32769 bytes in length or greater

Status: VERIFIED FIXED

(SeaMonkey :: General, defect)

Description

[Trent Bradley]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7) Gecko/20040803 Firefox/0.9.3
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7) Gecko/20040803 Firefox/0.9.3

On any page that has a textbox/textarea that has a value of 32769 bytes in
length or greater cause FireFox to crash.

Reproducible: Always
Steps to Reproduce:
1. Create a HTML page with a textbox/textarea
2. Fill it with a value of 32769 bytes in length or greater
3. Then open the HTML page in FireFox

Actual Results:  
FireFox crashed.

Expected Results:  
Not crashed.

    *  Mozilla FireFox 0.9.3
    * Extensions
          o Tabbrowser Extensions 1.11.2004081001
          o googlebar 0.9.0.16
          o Adblock 5 d2 nightly 39
          o BugMeNot 0.3
          o Allow Right-Click 0.1
          o ChatZilla 0.9.64b
    * Themes
          o Orbit Grey 0.2.1

Windows 98 SE 4.10.2222B
RAM 128 MB
CPU: 732 MHz

Comment 1

[Ryan Polk (Quark)]

WFM Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040809
Firefox/0.9.1+

0.9.2 build also worked.

This could be an OS version issue.

Comment 2

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Testcase WFM Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a3)
Gecko/20040807

I believe if this actually caused a crash, it should be marked security-sensitive

Comment 3

[Adam Hauner]

Attachment: testcase

Comment 4

[Adam Hauner]

WFM 2004080908/Seamonkey-trunk/W2K
WFM FF093/W2K

Trent Bradley: Could you provide TalkBack incident ID of your crash?

Comment 5

[Trent Bradley]

(In reply to comment #4)
> WFM 2004080908/Seamonkey-trunk/W2K
> WFM FF093/W2K
> 
> Trent Bradley: Could you provide TalkBack incident ID of your crash?

What's a "TalkBack"?

Comment 6

[Trent Bradley]

(In reply to comment #4)
> WFM 2004080908/Seamonkey-trunk/W2K
> WFM FF093/W2K
> 
> Trent Bradley: Could you provide TalkBack incident ID of your crash?

What's a "TalkBack"?

(In reply to comment #3)
> Created an attachment (id=155775)
> testcase
> 
Some reason that didn't cause FireFox to crash.... possibly because it was
different characters? When I tested it I used 32769 "a"s.

Comment 7

[Adam Hauner]

TalkBack is Mozilla Quality Feedback Agent. If your Firefox occur crash,
TalkBack will start and it'll provide you way to send report about crash
incidents. Go to your Firefox directory, then to its components subdirectory,
there you will find talkback.exe. Run it and you'll find list of IDs like TB545455.

Comment 8

[Trent Bradley]

(In reply to comment #7)
> TalkBack is Mozilla Quality Feedback Agent. If your Firefox occur crash,
> TalkBack will start and it'll provide you way to send report about crash
> incidents. Go to your Firefox directory, then to its components subdirectory,
> there you will find talkback.exe. Run it and you'll find list of IDs like
TB545455.

TalkBack doesn't start up when FireFox crashes... could this mean it's the OS
crashing, and not FireFox? Also, when I start TalkBack up before running FireFox
and then cause the crash, it records nothing.

Comment 9

[Trent Bradley]

Has this just been abandoned?

Comment 10

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Well, if nobody who looked at the bug can reproduce it, and we can't get a
Talkback ID to see where exactly the crash happened, nobody can fix the bug.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

WFM
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040803 Firefox/0.9.3

You need to get a talkback ID for this bug to go anywhere.

Comment 12

[Trent Bradley]

Ah, okay. Sorry for my ignorance, I've never reported a bug before... but the
talkback thing never launches when FireFox crashes... And if I start it up
before FireFox then cause it to crash... it still gives me nothing. It could
just be my computer, it is a P.O.S. just to let ya know.

Comment 13

[Felix Miata]

Using 1.7.2 zip build on 350 MHz PII W98SE I always get crash on
http://free.hostultra.com/~nerdonline/firefox/255120/example.html but apparently
talkback never sees it. I always get crash with no talkback on 0.9.3 zip and
today's trunk on K6/3+ 500 W98 1st as well. No crash with Linux trunk for me.

Comment 14

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

(In reply to comment #2)
> I believe if this actually caused a crash, it should be marked security-sensitive

biesi pointed out that it's highly unlikely to be a buffer overflow given that
it would imply someone used a 32kb static buffer.

Comment 15

[Hermann Schwab]

I would speculate that somewhere a 16 bit signed integer is used as index to the
bytes in the textbox. byte #1 is 0x0000, byte #32768 is 0x7fff, byte #32769 is
0x8000, a valid positve integer, an invalid negative integer, byte #32770 is
0x8001, the biggest negative integer in 16 bit representation. 
If 16 bit is expanded to 32 bit, 
0x8000 unsigned would be expanded to 0x00008000,
0x8000   signed would be expanded to 0xFFFF8000
0xFFFF8000 is dezimal -32768 seen as signed, or 4294934528 seen as unsigned.


http://lxr.mozilla.org/seamonkey/source/dom/public/idl/html/nsIDOMNSHTMLTextAreaElement.idl#46
...
 42 interface nsIControllers;
 43 
 44 
 45 [scriptable, uuid(ca066b44-9ddf-11d3-bccc-0060b0fc76bd)]
 46 interface nsIDOMNSHTMLTextAreaElement : nsISupports
 47 {
 48   readonly attribute nsIControllers   controllers;
 49 
 50   readonly attribute long    textLength;

type of  textLength is long, that´s a signed value, if not one of the modifiers
'readonly' od 'attribute' makes it unsigned.
If that long value is converted to a 16 bit int somewhere, it will be limited to
a positive value of 32767, any higher value will produce a negative number, if
converted back to long.

I´m not familiar with mozilla code, did just a LXR search for textarea.

Comment 16

[Daniel Veditz [:dveditz]]

Just tested both 0.9.1 and 0.10PR on a Win98 system. Both crash exactly as
described at the same address in GDI.EXE. I would guess this would happen in
Mozilla as well since textareas come from shared Gecko code.

This does not appear to be a buffer overrun in our code, but maybe we have to
detect Win9x and block excessive values. IE 5 does not crash on this page. I
didn't count the letters in the textbox to see if all 32K were there :-) or if
it truncated the data.

Comment 17

[chris hofmann]

dan,jst,others,  is there an easy fix here?

Comment 18

[Son Le]

It looks like we're going over the maximum text limit of a edit text control. 

From msdn: 
"On Windows 95/98/Me, for single-line edit controls, the text limit is 0x7FFE bytes"

I don't have win98 installed, but my guess is we need to change the code so that
it sets the text limit (EM_SETLIMITTEXT) and make sure we never set more text
(WM_SETTEXT) than the limit.

A stack signature would also help.

Comment 19

[Brian Ryner (not reading)]

Attachment: fix, of sorts

I think the problem here is that we're passing a string of arbitrary length to
the OS text measurement/drawing functions (GetTextExtentPoint, ExtTextOut), but
the MSDN docs state that the length may not exceed 8KB for Windows 95/98/ME. 
The best solution here would be for us to measure / draw the text in 8192-byte
chunks on those operating systems.  However, I don't feel confident enough in
my knowledge of these API's to make that patch, and I question whether it's
worth the risk for this edge case this close to Firefox 1.0.

I'm suggesting this fix instead, which simply drops the characters after 8192.

Comment 20

[rbs]

Comment on attachment 161929 [details] [diff] [review]
fix, of sorts

>The best solution here would be for us to measure / draw the text in 8192-byte
>chunks on those operating systems.

Note that you can get precisely this for free if you just do:

nsresult
nsFontMetricsWin::ResolveForwards(HDC		       aDC,
				  const PRUnichar*     aString,
				  PRUint32	       aLength,
				  nsFontSwitchCallback aFunc, 
				  void* 	       aData)
{
  NS_ASSERTION(aString || !aLength, "invalid call");
+ CheckLength(&aLength);
  const PRUnichar* firstChar = aString;
  const PRUnichar* currChar = firstChar;
  const PRUnichar* lastChar  = aString + aLength;
  [...]
}

nsresult
nsFontMetricsWin::ResolveBackwards(HDC			aDC,
				   const PRUnichar*	aString,
				   PRUint32		aLength,
				   nsFontSwitchCallback aFunc, 
				   void*		aData)
{
  NS_ASSERTION(aString || !aLength, "invalid call");
+ CheckLength(&aLength);
  const PRUnichar* firstChar = aString + aLength - 1;
  const PRUnichar* lastChar  = aString - 1;
  const PRUnichar* currChar  = firstChar;
  [...]
}

==============

The ultimate effet of this is that the respective callback functions (i.e.,
those helper GetWith, DrawString, etc) will be called again until the total
length is consumed.

However, this fix caters only for |PRUnichar*|. The |char *| functions are not
called back, and they will still need their own loop.

I am fine with this patch for the branch. But I recommend the complete fix for
the trunk.

Comment 21

[Jungshik Shin]

Comment on attachment 161929 [details] [diff] [review]
fix, of sorts

r=jshin with what rbs said.

Comment 22

[Brian Ryner (not reading)]

Comment on attachment 161929 [details] [diff] [review]
fix, of sorts

Requesting branch approval. My current plan is to _not_ check this in on the
trunk pending the more complete fix rbs suggested.  Sound ok?

Comment 23

[chris hofmann]

a=chofmann for the branches

ok on the trunk plan, but wish we had a 1.8a[next] flag to make sure we don't
forget...

Comment 24

[Brian Ryner (not reading)]

Adding fixed keywords, leaving open for trunk fix.

Comment 25

[rbs]

Stealing this from bryner... I will take a stab. On further thoughts, the fix
looks more involved than what I said earlier.

Comment 26

[Trent Bradley]

I must say good job guys, the new final release of 1.0 seems to have fixed the
problem, for me at least.

Comment 27

[Daniel Veditz [:dveditz]]

If nothing else we need the branch hack to land on the trunk.

Comment 28

[rbs]

I agree.

I looked at the problem and it was more invasive than initially thought. It is
not worth the hassle since it is only for those old and crusty Win9x boxes. Who
was I kidding? I am going to look at putting the hack either in
nsRenderingContextWin or nsFontMetricsWin, whichever is easier.

Comment 29

[rbs]

Attachment: Patch for the trunk

Same effect, but with the hack moved into nsRenderingConteWin where it can be
strategically placed next to |SetupFontAndColor| to catch all cases with fewer
calls.

Comment 30

[Jungshik Shin]

Comment on attachment 171099 [details] [diff] [review]
Patch for the trunk

r=jshin

Comment 31

[Hermann Schwab]

wfm Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b) Gecko/20050116
2nd testcase of bug 265857 also has a huge textarea, also wfm now.

Comment 32

[Hermann Schwab]

(In reply to comment #31)
> wfm Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b) Gecko/20050116
> 2nd testcase of bug 265857 also has a huge textarea, also wfm now.

STILL CRASHING

was testing testcase only, testcase from URL is still crashing:
the crashing link: (crashes)
http://free.hostultra.com/~nerdonline/firefox/255120/example.html

the URL: has some info, and the link above.
http://free.hostultra.com/~nerdonline/firefox/255120/

the original testcase has exactly 32769 chars in one line only, no whitespace.
the attached testcase contains 321 lines with 120 chars in  the textarea.

Comment 33

[Hermann Schwab]

Attachment: 32768 char prefilled textarea, add one char to crash on Win98

This testcase has a textarea prefilled to the max, place a cursor somewhere,
press any key which would add a byte, and enjoy the crash.
Talkback doesn´t come up, as it is a win98 internal crash, not related to
Mozilla.

Comment 34

[Hermann Schwab]

Attachment: 32769 char prefilled textarea, immediately crashing on Win98

body contains only a textarea containing exactly 32769 times the letter 'a'
the textarea doesn´t contain any whitespace, it is one line only in a
texteditor. 

I´m obsoleting attachment 155775 [details] as it doesn´t crash:
<textarea name="foo" cols="80" rows="20">
321 lines @120 chars each also containing some spaces.

Comment 35

[rbs]

This hasn't been fixed on trunk builds yet. I am going to check in soon. Try a
build that will have the fix to see if it still crashes.

Comment 36

[rbs]

Checked in the trunk.

Comment 37

[Hermann Schwab]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b) Gecko/20050119
tested on Win98 and Win98SE