Closed
Bug 1205163
Opened 9 years ago
Closed 8 years ago
<xsl:output method="text"/> triggers an assertion in debug builds.
Categories
(Core :: XSLT, defect)
Core
XSLT
Tracking
()
RESOLVED
FIXED
mozilla45
People
(Reporter: baku, Assigned: peterv)
References
Details
(Keywords: sec-low, Whiteboard: [post-critsmash-triage][adv-main45+])
Attachments
(2 files)
1.27 KB,
patch
|
Details | Diff | Splinter Review | |
3.24 KB,
patch
|
sicking
:
review+
|
Details | Diff | Splinter Review |
Soon a crashtest to reproduce this issue.
Reporter | ||
Updated•9 years ago
|
Group: dom-core-security
Reporter | ||
Comment 1•9 years ago
|
||
ASSERTION: Bad readystate: 'mDocument->IsXULDocument() || mDocument->GetReadyStateEnum() == nsIDocument::READYSTATE_INTERACTIVE || (mDocument->GetReadyStateEnum() == nsIDocument::READYSTATE_UNINITIALIZED && NS_IsAboutBlank(mDocument->GetDocumentURI()))', file /home/baku/Sources/m/sw/src/layout/base/nsDocumentViewer.cpp, line 974
Comment 2•9 years ago
|
||
Does that patch crash, or just assert? I think this might be a dup.
Reporter | ||
Comment 3•9 years ago
|
||
It could do a dup, I didn't actually check. The test asserts, and in debug builds it crashes.
Comment 4•9 years ago
|
||
Where does it crash? Since if there is a crash, that is something new.
Comment 5•9 years ago
|
||
(the assertion is NS_ASSERTION and that shouldn't crash by default)
Reporter | ||
Comment 6•9 years ago
|
||
You are right, it asserts.
Comment 7•9 years ago
|
||
How bad is this assertion?
Reporter | ||
Updated•9 years ago
|
Flags: needinfo?(peterv)
Assignee | ||
Comment 8•9 years ago
|
||
Looks like an oversight that this code wasn't added to txMozillaTextOutput (it was added in txMozillaXMLOutput). I looked into sharing code between them but it's not trivial.
Assignee: nobody → peterv
Status: NEW → ASSIGNED
Flags: needinfo?(peterv)
Attachment #8664777 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 9•9 years ago
|
||
And I have no idea if hitting the assertion is bad in this case.
Comment on attachment 8664777 [details] [diff] [review] v1 Review of attachment 8664777 [details] [diff] [review]: ----------------------------------------------------------------- Stealing. Feel free to ask bz for review as well if you want though.
Attachment #8664777 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 11•9 years ago
|
||
So is having the readyState enum be READYSTATE_UNINITIALIZED a security bug?
Flags: needinfo?(bzbarsky)
Comment 12•9 years ago
|
||
I don't actually know offhand. :( I want to say "no".
Flags: needinfo?(bzbarsky)
Comment 13•9 years ago
|
||
Peter: can this patch land? then we don't have to worry whether it's exploitable.
Flags: needinfo?(peterv)
Keywords: sec-low
Assignee | ||
Comment 14•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/407dff8daf62
Flags: needinfo?(peterv)
https://hg.mozilla.org/mozilla-central/rev/407dff8daf62
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Updated•8 years ago
|
Group: dom-core-security → core-security-release
Updated•8 years ago
|
Whiteboard: [post-critsmash-triage]
Updated•8 years ago
|
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main45+]
Updated•8 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•