After submitting a huge string in a textbox, profile becomes corrupted such that typing into any textbox hangs

VERIFIED FIXED

Status

()

Toolkit
Form Manager
VERIFIED FIXED
11 years ago
9 years ago

People

(Reporter: Marcel, Assigned: dveditz)

Tracking

(Blocks: 1 bug, 4 keywords)

unspecified
fixed1.8.0.12, fixed1.8.1.4, hang, perf
Points:
---
Bug Flags:
blocking1.8.1.4 +
wanted1.8.1.x +
blocking1.8.0.12 +
wanted1.8.0.x +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:low] persistent DoS [need branch approval], URL)

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

11 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; nl; rv:1.8.1) Gecko/20061010 Firefox/2.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; nl; rv:1.8.1) Gecko/20061010 Firefox/2.0

On a site, there was an inputfield. Well, in notepad i typed a lot of charachters, millions perhaps. I pasted this line in an input field and posted it to the server. It took so long that the browsers hanged. From that moment, i cannot type in any input field in my profile. On the first charachter, firefox hangs and i am forced to do a ctrl+alt+del. This problem does not happen when i use a new profile

Reproducible: Always

Steps to Reproduce:
1.Paste lots of text in an input field (so no textarea)
2.Post form
3.Restarted firefox
4.Do whatever, a search in google orso. My firefox (profile) hangs

Actual Results:  
Crash, not responding
(Reporter)

Comment 1

11 years ago
Did some more testing
I made a backup of my profile using mozbackup. I checked all except "saved form details". Then resored the backup, and now i dont have any problems now...

Maybe this additional information will help

Updated

11 years ago
Blocks: 123929
Severity: normal → critical
Keywords: crash, hang
Summary: Crash after input/post lots of text → Crash after input/post lots of text; profile becomes corrupted such that typing into any textbox hangs

Updated

11 years ago
Component: General → Form Manager
QA Contact: general → form.manager

Updated

10 years ago
Duplicate of this bug: 371436

Updated

10 years ago
Flags: blocking1.8.1.3?
Keywords: crash → perf
Summary: Crash after input/post lots of text; profile becomes corrupted such that typing into any textbox hangs → After submitting a huge string in a textbox, profile becomes corrupted such that typing into any textbox hangs
Whiteboard: [sg:low] persistent DoS
(Assignee)

Updated

10 years ago
Group: security
(Assignee)

Comment 3

10 years ago
I don't see how this can block a specific 1.8 release without some traction on a fix. Nominating for the trunk release and moving into the "wanted" bucket for the branch.

You hang on any input field? I was only able to reproduce hanging on input fields with the same name as my testcase (e.g. delay on Google, Yahoo still fine).
Severity: critical → normal
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.4?
Flags: blocking-firefox3?
(Assignee)

Comment 4

10 years ago
Created attachment 257780 [details]
testcase, induces delay at Google
(Assignee)

Comment 5

10 years ago
During the hang stacks tend to look like the following. Mork is growing the buffer 512 bytes at a time until it gets big enough to read the string value, which is going to take 32K allocs until it's big enough for the 16Mb string. I think the hope is to replace Mork, so meanwhile we should put a sane upper bound on saved form data of a couple of K.

>	memset() Line 108	Asm
 	_heap_alloc_dbg() Line 448	C
 	_nh_malloc_dbg() Line 260	C
 	_nh_malloc() Line 209	C
 	operator new() Line 25	C++
 	orkinHeap::Alloc() Line 90	C++
 	morkBlob::GrowBlob() Line 90	C++
 	morkCoil::GrowCoil() Line 164	C++
 	morkSpool::SpillPutc() Line 127	C++
 	morkSink::Putc() Line 132	C++
 	morkParser::ReadValue() Line 938	C++
 	morkParser::ReadCell() Line 587	C++
 	morkParser::ReadRow() Line 678	C++
 	morkParser::ReadTable() Line 789	C++
 	morkParser::ReadContent() Line 1432	C++
 	morkParser::OnPortState() Line 1473	C++
 	morkParser::ParseLoop() Line 1531	C++
 	morkParser::ParseMore() Line 1570	C++
 	morkThumb::DoMore_OpenFileStore() Line 479	C++
 	morkThumb::DoMore() Line 399	C++
 	morkThumb::DoMore() Line 194	C++
 	nsFormHistory::UseThumb() Line 588	C++
 	nsFormHistory::OpenExistingFile() Line 441	C++
 	nsFormHistory::OpenDatabase() Line 381	C++
 	nsFormHistory::AutoCompleteSearch() Line 722	C++
 	nsFormFillController::StartSearch() Line 556	C++
 	nsAutoCompleteController::StartSearch() Line 993	C++
 	nsAutoCompleteController::Notify() Line 688	C++
 	nsTimerImpl::Fire() Line 397	C++
 	nsTimerManager::FireNextIdleTimer() Line 628	C++
 	nsAppShell::Run() Line 141	C++
 	nsAppStartup::Run() Line 151	C++
 	XRE_main() Line 2695	C++
 	main() Line 61	C++
 	mainCRTStartup() Line 398	C

Cure once affected: use clear private data to delete form history.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: wanted1.8.0.x+
Flags: blocking1.8.1.4?
Flags: blocking1.8.0.12?

Comment 6

10 years ago
> Mork is growing the buffer 512 bytes at a time until it gets big enough to read the string value

Reminds me of bug 319004 comment 5.
(Assignee)

Comment 7

10 years ago
Apparently you need to quit and restart to get the full effect of the hang, the current 16Mb string version might be far too malicious. (note, mork actually saves the data as escaped UTF-16 so a single 'a' is saved as 'a$00' -- quadrupling the amount it needs to re-re-re-reallocate, 512 bytes at a time.)

If you edit the testcase to set the value to 's' instead of 's16' the resulting hang is bearable. It does happen on the first input field regardless of the site, but once the form data is loaded into history there aren't further delays.
(Assignee)

Updated

10 years ago
Assignee: nobody → dveditz
Flags: blocking1.8.1.4?
Flags: blocking1.8.1.4+
Flags: blocking1.8.0.12?
Flags: blocking1.8.0.12+

Updated

10 years ago
Flags: blocking1.8.1.4?
Flags: blocking1.8.1.4+
Flags: blocking1.8.0.12?
Flags: blocking1.8.0.12+
(Assignee)

Updated

10 years ago
Flags: blocking1.8.1.4?
Flags: blocking1.8.1.4+
Flags: blocking1.8.0.12?
Flags: blocking1.8.0.12+
Hmm, was this not fixed by bug 319004? Either way, we use mozstorage for form history on the trunk, so this bug doesn't apply there.
Flags: blocking-firefox3?
(Assignee)

Comment 9

10 years ago
No, it was not fixed by bug 319004. That hacked around the mork problem by limiting title strings specifically; it did not fix the underlying mork issue for other uses of mork. The branch fix I'm proposing here is a similar hack-around in the form-data code. The trunk "fix" is to stop using mork.

Not sure what Thunderbird/mailnews needs to worry about since they're still using mork on the trunk. A malicious vcard with arbitrary sized values? locally-saved IMAP mail?
(Assignee)

Comment 10

10 years ago
Created attachment 262627 [details] [diff] [review]
don't save form data that's too large

Prevent the branch DOS by limiting saved form data to 200 characters for the field ID and 2000 for the <input> text. Anything bigger and we simply don't save it. The limits are totally arbitrary, but it's hard to imagine a legit ID being 200 characters, or a 2000-char data string that you'd want to replay in future forms.

For the trunk I've kept the limits even though we now use mozstorage which doesn't have the performance suckage of mork. Large chunks of data still seem useless/malicious and not worth storing.
Attachment #262627 - Flags: review?(mano)
I don't think we should arbitrarily limit what we store on the trunk unless we have a good reason to.
Comment on attachment 262627 [details] [diff] [review]
don't save form data that's too large

What gavin said, plus  I would rather cut aValue at its maximum length rather than just throwing (for "invalid" name we should probably keep throwing, I guess).
Attachment #262627 - Flags: review?(mano) → review-
(Assignee)

Comment 13

10 years ago
The "good reason to" is because sites can store arbitrarily large amounts of data on disk. Although I guess even with a size limit they can just add more fields, so we'd really need a per-site quota of some sort (much like the cookie and globalStorage quotas). And there's no way to manage form data or even know how much you have other than periodically trashing it all. But OK, I'll leave the trunk alone and let that be a different issue (I'd still like to fix the potential uninitialized "exists" variable).

On the branch I don't see the value of saving truncated data. It's chopped off but you're not going to be able to tell that -- if users see the value as a choice they will naturally assume it's what they entered and could blame us for corrupted data. Much better to simply drop it (this is also the approach we take with overlarge cookies).

If you want larger limits I don't mind. 4K, 8K, 16K, whatever. Uselessly large but not yet close to causing the perf problem.
(Assignee)

Comment 14

10 years ago
Created attachment 262683 [details] [diff] [review]
fix mork-based history only

in nsFormHistory.cpp the return value doesn't matter since nothing checks it. Should someone want this in nsStorageFormHistory.cpp I'd probably want to return NS_OK so they just silently drop without bothering the caller.
Attachment #262627 - Attachment is obsolete: true
Attachment #262683 - Flags: review?(mano)
(Assignee)

Updated

10 years ago
Whiteboard: [sg:low] persistent DoS → [sg:low] persistent DoS [need r=mano, trunk landing, branch landing]
Comment on attachment 262683 [details] [diff] [review]
fix mork-based history only

>Index: toolkit/components/satchel/src/nsFormHistory.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/components/satchel/src/nsFormHistory.cpp,v
>retrieving revision 1.37
>diff -u -p -8 -r1.37 nsFormHistory.cpp
>--- toolkit/components/satchel/src/nsFormHistory.cpp	18 Jul 2006 00:18:32 -0000	1.37
>+++ toolkit/components/satchel/src/nsFormHistory.cpp	24 Apr 2007 20:54:18 -0000
>@@ -68,16 +68,20 @@ static void SwapBytes(PRUnichar* aDest, 
>     PRUnichar aChar = *aSrc++;
>     *aDest++ = (0xff & (aChar >> 8)) | (aChar << 8);
>   }
> }
> 
> #define PREF_FORMFILL_BRANCH "browser.formfill."
> #define PREF_FORMFILL_ENABLE "enable"
> 
>+// upper bounds on saved form data, more isn't useful.
>+#define FORMFILL_NAME_MAX_LEN  200
>+#define FORMFILL_VALUE_MAX_LEN 2000
>+

Sold, just increase those a bit (esp. the former); r=mano.
Attachment #262683 - Flags: review?(mano) → review+
(Assignee)

Comment 16

10 years ago
Created attachment 263092 [details] [diff] [review]
1.8/1.8.0 branch patch (lengths changed per review comments)
Attachment #263092 - Flags: approval1.8.1.4?
Attachment #263092 - Flags: approval1.8.0.12?
(Assignee)

Comment 17

10 years ago
Created attachment 263093 [details] [diff] [review]
ignore -- double submit
Attachment #263093 - Flags: approval1.8.1.4?
Attachment #263093 - Flags: approval1.8.0.12?
(Assignee)

Updated

10 years ago
Attachment #263093 - Attachment description: 1.8/1.8.0 branch patch (lengths changed per review comments) → ignore -- double submit
Attachment #263093 - Attachment is obsolete: true
Attachment #263093 - Flags: approval1.8.1.4?
Attachment #263093 - Flags: approval1.8.0.12?
(Assignee)

Comment 18

10 years ago
Checked into trunk
Status: NEW → RESOLVED
Last Resolved: 10 years ago
OS: Windows XP → All
Hardware: PC → All
Resolution: --- → FIXED
Whiteboard: [sg:low] persistent DoS [need r=mano, trunk landing, branch landing] → [sg:low] persistent DoS [need branch approval]
(Assignee)

Comment 19

10 years ago
Comment on attachment 263092 [details] [diff] [review]
1.8/1.8.0 branch patch (lengths changed per review comments)

approved for 1.8.0.12 and 1.8.1.4, a=release-drivers
(Assignee)

Updated

10 years ago
Attachment #263092 - Flags: approval1.8.1.4?
Attachment #263092 - Flags: approval1.8.1.4+
Attachment #263092 - Flags: approval1.8.0.12?
Attachment #263092 - Flags: approval1.8.0.12+
(Assignee)

Comment 20

10 years ago
fixed on 1.8 and 1.8.0 branches
Keywords: fixed1.8.0.12, fixed1.8.1.4
(Assignee)

Updated

10 years ago
Group: security
Duplicate of this bug: 378159

Comment 22

9 years ago
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1pre) Gecko/2008062306 GranParadiso/3.0.1pre

Verified.
Status: RESOLVED → VERIFIED
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.