Closed Bug 255120 Opened 20 years ago Closed 20 years ago

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

Categories

(SeaMonkey :: General, defect)

x86
Windows 98
defect
Not set
critical

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.8alpha5

People

(Reporter: trentmbradley, Assigned: rbs)

References

()

Details

(Keywords: crash, fixed-aviary1.0, fixed1.7.5, Whiteboard: Win9x only. need trunk)

Attachments

(3 files, 2 obsolete files)

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
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.
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
Severity: minor → critical
Attached file testcase (obsolete) —
WFM 2004080908/Seamonkey-trunk/W2K
WFM FF093/W2K

Trent Bradley: Could you provide TalkBack incident ID of your crash?
Keywords: crash
(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 #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.
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.
(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.
Has this just been abandoned?
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.
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.
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.
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.
Assignee: firefox → general
Status: UNCONFIRMED → NEW
Component: General → Browser-General
Ever confirmed: true
Product: Firefox → Browser
QA Contact: firefox.general → general
Version: unspecified → Trunk
(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.
Flags: blocking1.7.x?
Flags: blocking-aviary1.0?
Summary: If a textbox/textarea has a value of 32769 bytes in length or greater, FireFox crashes. → crash if a textbox/textarea has a value of 32769 bytes in length or greater
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.
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.
Whiteboard: Win9x only
dan,jst,others,  is there an easy fix here?
Assignee: general → dveditz
Flags: blocking-aviary1.0? → blocking-aviary1.0+
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.
Attached patch fix, of sorts (obsolete) — Splinter Review
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.
Assignee: dveditz → bryner
Status: NEW → ASSIGNED
Attachment #161929 - Flags: superreview?(rbs)
Attachment #161929 - Flags: review?(jshin)
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.
Attachment #161929 - Flags: superreview?(rbs) → superreview+
Comment on attachment 161929 [details] [diff] [review]
fix, of sorts

r=jshin with what rbs said.
Attachment #161929 - Flags: review?(jshin) → review+
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?
Attachment #161929 - Flags: approval1.7.x?
Attachment #161929 - Flags: approval-aviary?
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...
Flags: blocking1.7.x? → blocking1.7.x+
Attachment #161929 - Flags: approval1.7.x?
Attachment #161929 - Flags: approval1.7.x+
Attachment #161929 - Flags: approval-aviary?
Attachment #161929 - Flags: approval-aviary+
Adding fixed keywords, leaving open for trunk fix.
Stealing this from bryner... I will take a stab. On further thoughts, the fix
looks more involved than what I said earlier.
Assignee: bryner → rbs
Status: ASSIGNED → NEW
Target Milestone: --- → mozilla1.8alpha5
Product: Browser → Seamonkey
I must say good job guys, the new final release of 1.0 seems to have fixed the
problem, for me at least.
If nothing else we need the branch hack to land on the trunk.
Flags: blocking1.8b+
Whiteboard: Win9x only → Win9x only. need trunk
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.
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.
Attachment #161929 - Attachment is obsolete: true
Attachment #171099 - Flags: superreview?(bryner)
Attachment #171099 - Flags: review?(jshin)
Comment on attachment 171099 [details] [diff] [review]
Patch for the trunk

r=jshin
Attachment #171099 - Flags: review?(jshin) → review+
Attachment #171099 - Flags: superreview?(bryner) → superreview+
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.
(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.


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.
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.
Attachment #155775 - Attachment is obsolete: true
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.
Checked in the trunk.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b) Gecko/20050119
tested on Win98 and Win98SE
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: