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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox43 --- affected
firefox45 --- fixed

People

(Reporter: baku, Assigned: peterv)

References

Details

(Keywords: sec-low, Whiteboard: [post-critsmash-triage][adv-main45+])

Attachments

(2 files)

Soon a crashtest to reproduce this issue.
Group: dom-core-security
Attached patch crash.patchSplinter Review
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
Does that patch crash, or just assert? I think this might be a dup.
It could do a dup, I didn't actually check. The test asserts, and in debug builds it crashes.
Where does it crash? Since if there is a crash, that is something new.
(the assertion is NS_ASSERTION and that shouldn't crash by default)
You are right, it asserts.
How bad is this assertion?
Flags: needinfo?(peterv)
Attached patch v1Splinter Review
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)
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+
So is having the readyState enum be READYSTATE_UNINITIALIZED a security bug?
Flags: needinfo?(bzbarsky)
I don't actually know offhand.  :(  I want to say "no".
Flags: needinfo?(bzbarsky)
Peter: can this patch land? then we don't have to worry whether it's exploitable.
Flags: needinfo?(peterv)
Keywords: sec-low
https://hg.mozilla.org/mozilla-central/rev/407dff8daf62
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Group: dom-core-security → core-security-release
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main45+]
Group: core-security-release
Depends on: 1330492
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: