Closed
Bug 264956
Opened 20 years ago
Closed 20 years ago
Malformed HTML document causes crash - M173 FF10PR1 [@ RuleProcessorData::RuleProcessorData]
Categories
(Core :: DOM: HTML Parser, defect)
Core
DOM: HTML Parser
Tracking
()
VERIFIED
FIXED
People
(Reporter: wd, Assigned: mrbkap)
References
()
Details
(4 keywords, Whiteboard: [sg:dos])
Crash Data
Attachments
(2 files, 5 obsolete files)
1.85 KB,
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
3.21 KB,
patch
|
mkaply
:
review+
bzbarsky
:
superreview+
mkaply
:
approval-aviary+
mkaply
:
approval1.7.5+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20040910 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20040910 Visit the URL for this bug. Mozilla crashes. Reproducible: Always Steps to Reproduce: 1.Go to http://lcamtuf.coredump.cx/mangleme/gallery/mozilla_die1.html 2. 3. Actual Results: Crash Expected Results: No crash http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB1379160K http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB1380495E
Comment 1•20 years ago
|
||
see also bug 264944... that one seems a bit broader, marking dependency
Blocks: Zalewski
Assignee | ||
Comment 2•20 years ago
|
||
Allows NUL characters to stop our parsing of "tag identifiers". Also strips these characters out of the input stream (at least after tag identifiers) since attributes have a field day with these NULs (and we *really* don't want NULs floating around attribute code).
Assignee: parser → mrbkap
Status: NEW → ASSIGNED
Assignee | ||
Updated•20 years ago
|
Attachment #162530 -
Flags: superreview?(bzbarsky)
Attachment #162530 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 3•20 years ago
|
||
Updated to review comments.
Attachment #162530 -
Attachment is obsolete: true
Updated•20 years ago
|
Attachment #162530 -
Flags: superreview?(bzbarsky)
Attachment #162530 -
Flags: superreview-
Attachment #162530 -
Flags: review?(bzbarsky)
Attachment #162530 -
Flags: review-
Comment 4•20 years ago
|
||
Comment on attachment 162536 [details] [diff] [review] patch for checkin r+sr=bzbarsky
Attachment #162536 -
Flags: superreview+
Attachment #162536 -
Flags: review+
Assignee | ||
Comment 5•20 years ago
|
||
Fix checked in: Checking in src/nsScanner.cpp; /cvsroot/mozilla/parser/htmlparser/src/nsScanner.cpp,v <-- nsScanner.cpp new revision: 3.136; previous revision: 3.135 done
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Comment 6•20 years ago
|
||
F10PR and actual Firefox branch nightbuild is also crashing: http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB1389820E http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB1389919E M1.7 on branch will probably also crash. mkaply, chofmann: could you consider preparing patch for branches, as this bug was published in Bugtraq <http://www.securityfocus.com/archive/1/378632/2004-10-15/2004-10-21/0>?
Comment 7•20 years ago
|
||
I'll certainly take this for the branches. Does it apply as is?
Comment 8•20 years ago
|
||
*** Bug 265102 has been marked as a duplicate of this bug. ***
mkaply@us.ibm.com: no it does not apply to 17 branch as is. the file lives in a different place and the code is different.
Comment 10•20 years ago
|
||
Yeah, 1.7 code isn't even close. Anyone want to volunteer?
Assignee | ||
Comment 11•20 years ago
|
||
I'm not sure if this patch even compiles, but this is the last patch backported to the 1.7 branch (I hope). Please test to make sure this works.
Assignee | ||
Comment 12•20 years ago
|
||
Sorry, got the wrong file last time. Also included a change to a function I missed earlier. This needs testing, also.
Assignee | ||
Comment 13•20 years ago
|
||
Sorry, got the wrong file last time. Also included a change to a function I missed earlier. This needs testing, also.
Attachment #162629 -
Attachment is obsolete: true
Assignee | ||
Updated•20 years ago
|
Attachment #162632 -
Attachment is obsolete: true
Comment 14•20 years ago
|
||
marking blocking 1.7x, presuming aviary will follow suit and marking so.
Flags: blocking1.7.x+
Flags: blocking-aviary1.0+
Updated•20 years ago
|
Whiteboard: [sg:dos] have patch, need branch reviews
Comment 15•20 years ago
|
||
*** Bug 265206 has been marked as a duplicate of this bug. ***
Comment 16•20 years ago
|
||
The last 1.7 branch patch was missing the closing "}" for "if (theChar)" which I added after "current++;". With this patch a build of the current 1.7 branch does not crash any more, but instead it hangs with 100% CPU usage on the testcase page [Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.7.3) Gecko/20041020].
Attachment #162633 -
Attachment is obsolete: true
Comment 17•20 years ago
|
||
That was stupid of course, the "if" was meant to go as I could have seen in the patch for the trunk. With this one it works fine for a 1.7 branch build on OS/2.
Attachment #162681 -
Attachment is obsolete: true
Comment 18•20 years ago
|
||
Comment on attachment 162686 [details] [diff] [review] removed the "if (theChar)" from the 1.7 branch patch r=mkaply a=mkaply for branches
Attachment #162686 -
Flags: superreview?(bzbarsky)
Attachment #162686 -
Flags: review+
Attachment #162686 -
Flags: approval1.7.x+
Attachment #162686 -
Flags: approval-aviary+
Comment 19•20 years ago
|
||
Comment on attachment 162686 [details] [diff] [review] removed the "if (theChar)" from the 1.7 branch patch sr=bzbarsky.
Attachment #162686 -
Flags: superreview?(bzbarsky) → superreview+
Updated•20 years ago
|
Whiteboard: [sg:dos] have patch, need branch reviews → [sg:dos] have patch, needs branch landing
Comment 20•20 years ago
|
||
Note that another (less correct) fix for this was checked in on the branches in bug 265068... (which was security-sensitive, so we had no idea it existed).
Comment 21•20 years ago
|
||
Why is this bug "resolved fixed"??? This patch went nowhere. Certainly neither to the trunk, nor to the 1.7 branch. The thing checked in to the trunk is the much less comprehensive patch from bug 265068 which actually conflicts with this patch (it should be backed out first as noted in comment 265068#13) I do not reopen yet but if nothing happens soon...
Assignee | ||
Comment 22•20 years ago
|
||
(In reply to comment #21) > Why is this bug "resolved fixed"??? This patch went nowhere. Certainly neither > to the trunk, nor to the 1.7 branch. The patch for the trunk was checked in by me, as I said in comment 5. If you want to reopen until the patch for the branches is checked in (which will probably be the moment the tree is reopened from the smoketest blockers), feel free to.
Assignee | ||
Comment 23•20 years ago
|
||
(In reply to comment #22) > The patch for the trunk was checked in by me, as I said in comment 5. If you > want to reopen until the patch for the branches is checked in (which will > probably be the moment the tree is reopened from the smoketest blockers), feel > free to. Sorry for the bugspam. I misspoke the avairy tree is *not* closed. This patch will probably be checked in once the patch from bug 265068 is backed out.
Comment 24•20 years ago
|
||
Fix landed on branches. Too bad I didn't find this bug before landing the fix for bug 265068 :(
Keywords: fixed-aviary1.0,
fixed1.7
Comment 25•20 years ago
|
||
note: keyword for current 1.7 branch is fixed1.7.x. fixed1.7 was for bugs fixed in the 1.7.0 release. note2: bugs are not usually reopened just because they aren't landed on some branch yet. the FIXED resolution applies to trunk.
Keywords: fixed1.7 → fixed1.7.x
Comment 26•20 years ago
|
||
OK. Sorry. I (wrongly) thought the trunk patch was not applied. I did not check far back enough :-( But I did not reopen anyway :-)
Comment 27•20 years ago
|
||
(In reply to comment #17) > Created an attachment (id=162686) > removed the "if (theChar)" from the 1.7 branch patch I'm sorry if this is spam, but is it ok the patch for the trunk covers only the first half of this branch patch? E.g. it doesn't remove the if(theChar) in ReadEntityIdentifier().
Comment 28•20 years ago
|
||
ReadEntityIdentifier() doesn't exist on the trunk, so AFAICT, it's all good...
Comment 29•20 years ago
|
||
(In reply to comment #28) > ReadEntityIdentifier() doesn't exist on the trunk, so AFAICT, it's all good... Ahem, I meant to say "aviary branch" there, not "trunk".
Updated•20 years ago
|
OS: Windows XP → All
Hardware: PC → All
Comment 30•20 years ago
|
||
*** Bug 265966 has been marked as a duplicate of this bug. ***
Comment 31•20 years ago
|
||
*** Bug 265946 has been marked as a duplicate of this bug. ***
Comment 32•20 years ago
|
||
Verifying this fix per Talkback data (no crashes since 20041020 builds) and the fact that the repro testcase no longer crashes. Adding topcrash info for future reference.
Status: RESOLVED → VERIFIED
Keywords: topcrash
Summary: Malformed HTML document causes crash [@RuleProcessorData::RuleProcessorData] → Malformed HTML document causes crash - FF10PR1 [@ RuleProcessorData::RuleProcessorData]
Comment 33•20 years ago
|
||
(In reply to comment #32) > Verifying this fix per Talkback data (no crashes since 20041020 builds) and the > fact that the repro testcase no longer crashes. Adding topcrash info for future > reference. > On my new installation of Mozilla/5.0 (X11; U; Linux i686; rv:1.7.3) Gecko/20041026 Firefox/0.10.1 (Debian package 0.10.1+1.0PR-4) it still crush...
Comment 34•20 years ago
|
||
(In reply to comment #33) > (In reply to comment #32) > > Verifying this fix per Talkback data (no crashes since 20041020 builds) and the > > fact that the repro testcase no longer crashes. Adding topcrash info for future > > reference. > > > On my new installation of Mozilla/5.0 (X11; U; Linux i686; rv:1.7.3) > Gecko/20041026 Firefox/0.10.1 (Debian package 0.10.1+1.0PR-4) > > it still crush... > Replaying for myself... Afther longer investigation, i found that the last upstream for the source was in Oct 3. So my previous report may not be valid.
Comment 35•20 years ago
|
||
Re: comment 34 The curse of Gecko dates being build dates rather than pull dates strikes again :-( The true 20041026 builds should be labelled Firefox/1.0RC1 I swear I'm going to fix that one of these days
Whiteboard: [sg:dos] have patch, needs branch landing → [sg:dos]
Comment 36•20 years ago
|
||
(In reply to comment #35) > Re: comment 34 > > The curse of Gecko dates being build dates rather than pull dates strikes again > :-( The true 20041026 builds should be labelled Firefox/1.0RC1 > > I swear I'm going to fix that one of these days On the new update of: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.3) Gecko/20041029 that was made from the upstrem Thu, 28 Oct 2004, I can confirm that the bug seems to be fixed
Comment 37•20 years ago
|
||
Adding M173 since this was a topcrasher for Mozilla 1.7.3 and to make sure I verify this is gone once 1.7.5 is out the door.
Summary: Malformed HTML document causes crash - FF10PR1 [@ RuleProcessorData::RuleProcessorData] → Malformed HTML document causes crash - M173 FF10PR1 [@ RuleProcessorData::RuleProcessorData]
Updated•13 years ago
|
Crash Signature: [@ RuleProcessorData::RuleProcessorData]
You need to log in
before you can comment on or make changes to this bug.
Description
•