Closed Bug 319846 Opened 16 years ago Closed 15 years ago
overlong data in localstore
.rdf causes Do S on startup (persistent)
684 bytes, application/vnd.mozilla.xul+xml
1.20 KB, patch
|Details | Diff | Splinter Review|
This is similar to Bug 319004. Steps to Reproduce: 1. Load testcase, and click "Click me!" button. 2. Restart the browser. The browser will become unresponsive for a few minutes on every startup. I'm not sure if this needs to be marked as security-sensitive bug, or not.
Seems to spend its time in little2_scanAtts (xmltok_impl.c), at least that's where it always seems to be when I break in the debugger. No profiler ATM so can't get much more specific. Looks like it reparses the whole millions-of-chars line after every 4K chunk is appended and found to still be incomplete. The safe, quick fix is to probably put a limit on the size of persisted attributes (bug 295994 was going to put some limits by virtue of switching back-ends). We probably have to do something about the XML parser as well, it'd be very easy for someone to serve up an XML doc with really long attribute values of repeated characters. If served compressed these would download quickly and hang the user for an arbitrary amount of time.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: overlong data in localstore.rdf causes DoS on startup → overlong data in localstore.rdf causes DoS on startup (persistent)
Clamping persisted attributes to 2k sounds reasonable, and can be done at http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/xul/document/src/nsXULDocument.cpp&rev=1.682&mark=1427#1424
This solves the persistent part of the DoS by clamping attribute values at 4k (picked over 2k because it matches the length that will be imposed when the storage back-end is reimplemented in bug 295994). Does not touch the n^2 issue in the xml tokenizer.
File a followup bug on the XML parser issue and cc peterv on it?
Comment on attachment 210884 [details] [diff] [review] clamp attribute length to 4k I think that this patch might clamp in the middle of a non-BMP character, but other than that, it looks fine (and I'm not sure that this is even a problem).
Attachment #210884 - Flags: review?(mrbkap) → review+
Comment on attachment 210884 [details] [diff] [review] clamp attribute length to 4k a=timr for drivers
Fixed on trunk and 1.7, aviary101, and 1.8 branches. Filed bug 326206 on the parser issue.
verified with: Windows: Moz - Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.13) Gecko/20060214 Fx - Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.13) Gecko/20060214 Firefox/1.0.8
Comment on attachment 210884 [details] [diff] [review] clamp attribute length to 4k approved for 1.8.0 branch, a=dveditz for drivers
Attachment #210884 - Flags: approval22.214.171.124? → approval126.96.36.199+
Marking [rft-dl] (ready for testing in Firefox 188.8.131.52 release candidates) since in-testsuite+ indicates a test case exists in the js test library.
Whiteboard: [sg:dos] → [sg:dos][rft-dl]
You need to log in before you can comment on or make changes to this bug.