Closed
Bug 528300
Opened 15 years ago
Closed 15 years ago
Crash [@ txExecutionState::~txExecutionState]
Categories
(Core :: XSLT, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9.2
Tracking | Status | |
---|---|---|
status1.9.2 | --- | beta4-fixed |
blocking1.9.1 | --- | .8+ |
status1.9.1 | --- | .8-fixed |
People
(Reporter: pvnick, Assigned: peterv)
References
Details
(4 keywords, Whiteboard: [sg:critical?])
Crash Data
Attachments
(4 files, 3 obsolete files)
464 bytes,
text/xml
|
Details | |
1.44 KB,
text/plain
|
Details | |
8.60 KB,
patch
|
sicking
:
review+
dveditz
:
approval1.9.1.8+
dveditz
:
approval1.9.0.18+
|
Details | Diff | Splinter Review |
7.87 KB,
patch
|
Details | Diff | Splinter Review |
Load the testcase and refresh if it doesn't crash right away. EIP is sometimes different, especially when extra fluff is added to the transform and to the original document, so IMO this is exploitable.
Reporter | ||
Comment 1•15 years ago
|
||
Reporter | ||
Comment 2•15 years ago
|
||
WFM unless I save the file locally then open it.
Updated•15 years ago
|
Attachment #412043 -
Attachment description: testcase → testcase (don't load from bugzilla)
Updated•15 years ago
|
Reporter | ||
Updated•15 years ago
|
Blocks: fuzz-xslt-xpath
Assignee | ||
Comment 3•15 years ago
|
||
Still need to add the crashtest.
Assignee: nobody → peterv
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•15 years ago
|
||
Attachment #412409 -
Attachment is obsolete: true
Assignee | ||
Comment 6•15 years ago
|
||
I'll also add the crashtest from bug 528488.
Assignee | ||
Comment 7•15 years ago
|
||
Attachment #412413 -
Attachment is obsolete: true
Attachment #412490 -
Flags: review?(jonas)
Reporter | ||
Comment 8•15 years ago
|
||
Comment on attachment 412490 [details] [diff] [review]
v1.1
I don't crash on this testcase or the one in bug 528488 with the patch applied.
Attachment #412490 -
Flags: review+
Comment on attachment 412490 [details] [diff] [review]
v1.1
So why exactly are we crashing? Is the global-variable code ending up in a bad state or something?
While this patch seems like the right thing to do, I'm wondering if we could/should secure more code.
r=me anyhow
Attachment #412490 -
Flags: review?(jonas) → review+
Assignee | ||
Comment 10•15 years ago
|
||
We bail out early because of an error when trying to get the variable in txExecutionState::getVariable. We don't pop the result handler from the stack. We have code to deal with that, not deleting the unknown handler if an error happens (since the result handler stack will delete it). But evaluateToNumber eats the error return and so we do delete the unknown handler, and then the stacks tries to do that again.
Ah, and we can't pop these as we go up the callstack because it was push by a txInstruction and so we can't pop these guys off as we unwind.
Would be nice to get this more stable, but I can't think of a way to do so for now.
Assignee | ||
Comment 12•15 years ago
|
||
Need this crash fix on branch (not yet landed on trunk, need blocking or approval first).
Flags: blocking1.9.2?
Updated•15 years ago
|
Flags: blocking1.9.2? → blocking1.9.2+
Assignee | ||
Comment 13•15 years ago
|
||
Turns out the testcase for bug 528488 exposed a small leak in txExecutionState::getVariable. This is what I landed.
Attachment #412490 -
Attachment is obsolete: true
Attachment #413940 -
Flags: review?(jonas)
Assignee | ||
Comment 14•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/a727327f9777
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/74188d9e9faf
I'll nominate for other branches too.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
status1.9.2:
--- → final-fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2
Comment on attachment 413940 [details] [diff] [review]
v1.2
Would be nice if you could diff with -u8
r=me
Attachment #413940 -
Flags: review?(jonas) → review+
Assignee | ||
Comment 16•15 years ago
|
||
Comment on attachment 413940 [details] [diff] [review]
v1.2
XSLT crash fix.
Attachment #413940 -
Flags: approval1.9.1.7?
Attachment #413940 -
Flags: approval1.9.0.17?
Updated•15 years ago
|
Comment 17•15 years ago
|
||
Comment on attachment 413940 [details] [diff] [review]
v1.2
Approved for 1.9.1.7 and 1.9.0.17, a=dveditz for release-drivers
Attachment #413940 -
Flags: approval1.9.1.7?
Attachment #413940 -
Flags: approval1.9.1.7+
Attachment #413940 -
Flags: approval1.9.0.17?
Attachment #413940 -
Flags: approval1.9.0.17+
Assignee | ||
Comment 18•15 years ago
|
||
Keywords: fixed1.9.0.17
Comment 19•15 years ago
|
||
Verified fixed in 1.9.1 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.8pre) Gecko/20100115 Shiretoko/3.5.8pre (.NET CLR 3.5.30729). Attached testcase crashes in 1.9.1.7 but just shows "Error during XSLT transformation: XSLT Stylesheet (possibly) contains a recursion." in the 1.9.1 current nightly.
Keywords: verified1.9.1
Comment 20•15 years ago
|
||
Affects 1.8.0
Comment 21•15 years ago
|
||
Verified for 1.9.0.18 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.16pre) Gecko/2010020215 GranParadiso/3.0.18pre (.NET CLR 3.5.30729) with attached testcase. Results are the same as the 1.9.1.8 verification with a crash in 1.9.0.17 but an XSLT error in 1.9.0.18.
Keywords: fixed1.9.0.18 → verified1.9.0.18
Updated•15 years ago
|
Group: core-security
Updated•14 years ago
|
Crash Signature: [@ txExecutionState::~txExecutionState]
You need to log in
before you can comment on or make changes to this bug.
Description
•