Closed
Bug 590291
Opened 14 years ago
Closed 14 years ago
Crash [@ nsSVGGlyphFrame::GetExtentOfChar] with getExtentOfChar(0)
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | final+ |
blocking1.9.2 | --- | needed |
status1.9.2 | --- | .11-fixed |
status1.9.1 | --- | unaffected |
People
(Reporter: martijn.martijn, Assigned: longsonr)
References
(Blocks 1 open bug)
Details
(4 keywords, Whiteboard: [sg:critical?])
Crash Data
Attachments
(2 files)
243 bytes,
image/svg+xml
|
Details | |
1.17 KB,
patch
|
dholbert
:
review+
dveditz
:
approval1.9.2.11+
|
Details | Diff | Splinter Review |
See testcase, which crashes current trunk build and Firefox3.6.8. http://crash-stats.mozilla.com/report/index/bp-c88327df-4ec8-4970-8c25-fe09d2100824 0 xul.dll nsSVGGlyphFrame::GetExtentOfChar layout/svg/base/src/nsSVGGlyphFrame.cpp:1077 1 xul.dll nsSVGTextContainerFrame::GetExtentOfChar layout/svg/base/src/nsSVGTextContainerFrame.cpp:195 2 xul.dll nsSVGTextElement::GetExtentOfChar content/svg/content/src/nsSVGTextElement.cpp:324 3 xul.dll NS_InvokeByIndex_P xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102 4 xul.dll js::InvokeCommon<int > js/src/jsinterp.cpp:566 5 xul.dll js::Invoke js/src/jsinterp.cpp:699 6 xul.dll js::Interpret js/src/jsinterp.cpp:4709 7 xul.dll js::Execute js/src/jsinterp.cpp:883 8 xul.dll JS_EvaluateUCScriptForPrincipals js/src/jsapi.cpp:4769 9 xul.dll nsJSContext::EvaluateString dom/base/nsJSEnvironment.cpp:1811 10 xul.dll nsScriptLoader::EvaluateScript content/base/src/nsScriptLoader.cpp:767 11 xul.dll nsScriptLoader::ProcessRequest content/base/src/nsScriptLoader.cpp:677 12 xul.dll nsScriptLoader::ProcessScriptElement content/base/src/nsScriptLoader.cpp:617 13 xul.dll nsScriptElement::MaybeProcessScript content/base/src/nsScriptElement.cpp:195 14 xul.dll nsSVGScriptElement::DoneAddingChildren content/svg/content/src/nsSVGScriptElement.cpp:280 15 xul.dll nsXMLContentSink::CloseElement etc..
Comment 1•14 years ago
|
||
Crashes on Mac as well, changing to all.
OS: Windows 7 → All
Hardware: x86 → All
Assignee: nobody → jwatt
Updated•14 years ago
|
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
blocking2.0: --- → ?
Updated•14 years ago
|
blocking1.9.1: ? → needed
blocking1.9.2: ? → needed
status1.9.1:
--- → wanted
status1.9.2:
--- → wanted
Comment 2•14 years ago
|
||
Jwatt, any updates here? Marking sg:critical? until we learn more about the reason for this crash.
Whiteboard: [sg:critical?]
Comment 4•14 years ago
|
||
I think this is an sg:dos null deref. The crash-stacks on windows point at an uninteresting line 1077, but I think the optimizer is playing games with the loops. In mac debug and opt builds build I crash just after calling GetLength() on a null mTextRun. I don't understand why I don't crash calling mTextRun->IsClusterStart(limit) first though. In the debug build it ran quite a long time before crashing. Is it possible we're reusing a dead object that just happens to have nulls in an opt build?
Comment 5•14 years ago
|
||
Doesn't crash 1.9.0 or 1.9.1. The code that's crashing was added in bug 478792 http://hg.mozilla.org/mozilla-central/rev/f6cdd2d6a9ea
Assignee | ||
Comment 6•14 years ago
|
||
the CharacterIterator tries to create the mTextRun so that needs to come first. In this case even that does not set a mTextRun since we go into an error state with mInError = true. Chicken and egg at first sight since you need to advance to detect the error and you need a text run to figure out where to advance to but you can just advance to the beginning of the string and then figure out whether you need to advance any more later.
Attachment #471012 -
Flags: review?(dholbert)
Updated•14 years ago
|
Attachment #471012 -
Flags: review?(dholbert) → review+
Assignee | ||
Comment 7•14 years ago
|
||
Is that it for me in the brave new world of security fixes being checked in by the security group? The patch should apply to both trunk and 1.9.2 though you may need to fuzz things a little for the branch.
blocking2.0: ? → final+
Comment 8•14 years ago
|
||
Comment on attachment 471012 [details] [diff] [review] patch Approved for 1.9.2.10, a=dveditz Please land this fix on trunk and the 1.9.2 branch.
Attachment #471012 -
Flags: approval1.9.2.10+
Comment 9•14 years ago
|
||
(We're not really using the private repositories yet.)
Assignee | ||
Comment 10•14 years ago
|
||
pushed http://hg.mozilla.org/mozilla-central/rev/29b9772d0c3a pushed http://hg.mozilla.org/releases/mozilla-1.9.2/rev/8d2f5d3141c7
Assignee: jfkthame → longsonr
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 11•14 years ago
|
||
Verified fixed for 1.9.2 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.11pre) Gecko/20100921 Namoroka/3.6.11pre. Verified crash with testcase in 1.9.2.10.
Keywords: verified1.9.2
Comment 12•14 years ago
|
||
I see that this shipped, but without any credit for cross_fuzz? Bummer :-(
I think Martijn, the original reporter, wasn't using cross_fuzz.
Oh, he was. Sorry.
Comment 15•14 years ago
|
||
Is cross_fuzz public? I thought it wasn't and I don't know with whom you've shared it. Didn't want to start a deluge of "why won't you share it with _me_?" requests headed your way. Should we add a note like the ones in MFSA 2010-49 and MFSA 2010-59 every time we fix a crossfuzz-found bug? Forever? Until the fuzzer is public?
Comment 16•14 years ago
|
||
It's not public, it's shared with all the major browser vendors (as all are affected). I certainly don't mind a deluge of requests (if there's one thing I'm good at, it's turning people down). It's no big deal, really; it'd be cool to get a customary acknowledgment on fixed bugs, as it's pretty much the currency in the open source infosec world - but it's your call, depending on how much you believe cross_fuzz actually contributed to spotting the flaw, etc.
Updated•14 years ago
|
Group: core-security
Comment 17•14 years ago
|
||
Crashtest: http://hg.mozilla.org/mozilla-central/rev/d29ac45571d9
Flags: in-testsuite+
Updated•13 years ago
|
Crash Signature: [@ nsSVGGlyphFrame::GetExtentOfChar]
You need to log in
before you can comment on or make changes to this bug.
Description
•