Closed Bug 485217 (CVE-2009-1169) Opened 12 years ago Closed 12 years ago
Exploitable crash in [@tx
Mozilla XSLTProcessor::Transform To Doc ]
10.00 KB, application/x-tar
1.23 KB, patch
|Details | Diff | Splinter Review|
1.39 KB, patch
|Details | Diff | Splinter Review|
1.48 KB, patch
|Details | Diff | Splinter Review|
Found on security focus, not sure where the original came from. Exploit code at the link iframes a little xml file with an xslt transform that causes a crash reliably on 3.0 branch and trunk (and presumably 1.9.1, didn't test). Null, but it's being called, assuming the worst for the moment. Mac 3.0 crash report: http://crash-stats.mozilla.com/report/index/73cd82a1-6465-4ca2-9467-ef4982090325?p=1
No reason to believe it's mac-specific.
OS: Mac OS X → All
Yeah, crashes on linux too.
Obvious bug once you see it -- we can't leave this particular eval context on the context stack for the execution state to clean up.
Whiteboard: [sg:critical?] → [sg:critical]
Summary: Possibly exploitable crash in xMozillaXSLTProcessor::TransformToDoc → Exploitable crash in xMozillaXSLTProcessor::TransformToDoc
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P1
Target Milestone: --- → mozilla1.9.1b4
Original source appears to be http://milw0rm.com/exploits/8285 credited to Guido Landi http://milw0rm.com/author/1413 Not much point in keeping this bug hidden
http://hg.mozilla.org/mozilla-central/rev/bc263bec4a3e I'll check this into the 1.9.5 branch once it's green again.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment on attachment 369321 [details] [diff] [review] Fix This applies cleanly to the 1.9.0 branch.
Attachment #369321 - Flags: approval184.108.40.206?
Note for the 1.8 branch: this bug exists there, but the file moved from extensions/transformiix/source/xslt/functions/txKeyFunctionCall.cpp.
Comment on attachment 369321 [details] [diff] [review] Fix Approved for 220.127.116.11. a=ss for release-drivers. Please land immediately on 1.9.0.
Attachment #369321 - Flags: approval18.104.22.168? → approval22.214.171.124+
Just for the record, CVS trunk: mozilla/content/xslt/src/xslt/txKeyFunctionCall.cpp 1.42
This patch is for the 1.8.1 branch. I don't have a 1.8.0 tree to see if it applies there too.
Attachment #369357 - Flags: approval1.8.1.next? → approval1.8.1.next+
Comment on attachment 369357 [details] [diff] [review] Fix for the 1.8.1 branch Approved for 126.96.36.199. a=ss
I'm checking this in everywhere now.
http://hg.mozilla.org/mozilla-central/rev/6586181d850b and http://hg.mozilla.org/releases/mozilla-1.9.1/rev/291e1fee55b7 contain crashtests. /cvsroot/mozilla/extensions/transformiix/source/xslt/functions/Attic/txKeyFunctionCall.cpp,v <-- txKeyFunctionCall.cpp new revision: 188.8.131.52; previous revision: 1.33 is on the 1.8.1 branch.
Comment on attachment 369357 [details] [diff] [review] Fix for the 1.8.1 branch This applies cleanly to the 1.8.0 branch
Attachment #369357 - Flags: approval1.8.0.next+
Macro-obscured return combined with manual storage management considered harmful. Wish for static analysis to find such bad patterns, but it seems hard. /be
12 years ago
Hardware: x86 → All
Target Milestone: mozilla1.9.1b4 → mozilla1.9.2a1
Version: unspecified → Trunk
12 years ago
Flags: blocking1.8.0.next? → blocking1.8.0.next+
(In reply to comment #17) > Macro-obscured return combined with manual storage management considered > harmful. Wish for static analysis to find such bad patterns, but it seems hard. > > /be This does seem to be superficially similar to push/pop analysis I did in spidermonkey. If we had some heuristic to decide when these patterns are function-local, might be able to do scan for them.
Summary: Exploitable crash in xMozillaXSLTProcessor::TransformToDoc → Exploitable crash in [@txMozillaXSLTProcessor::TransformToDoc ]
Duplicate of this bug: 485382
Verified fixed on: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:184.108.40.206) Gecko/2009032609 Firefox/3.0.8 (.NET CLR 3.5.30729) Mozilla/5.0 (X11; U; Linux i686; en-US; rv:220.127.116.11) Gecko/2009032608 Firefox/3.0.8 Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:18.104.22.168) Gecko/2009032608 Firefox/3.0.8 Fx307 in all three platforms crashed with the test case here and the test case in bug 460090. It did not crash on Fx308.
Bug 460090 comment 8 contains a fix, reasoning, and tests - that I posted back in November. Clearly I don't deserve my name in the commit history for failing - over the course of four months - to navigate the review process to move one line of code up one space. My apologies.
(In reply to comment #22) > Bug 460090 comment 8 contains a fix, reasoning, and tests - that I posted back > in November. > > Clearly I don't deserve my name in the commit history for failing - over the > course of four months - to navigate the review process to move one line of code > up one space. My apologies. Passive-aggressive insinuations of bad faith generally don't win friends and influence people. That said, you're in luck, because I guess I'm just weird. I started a thread in mozilla.governance about this to figure out what we can do better on a variety of different levels; your feedback would be appreciated. http://groups.google.com/group/mozilla.governance/browse_thread/thread/f01ea12ff3c36522
(In reply to comment #22) > Clearly I don't deserve my name in the commit history for failing - over the > course of four months - to navigate the review process to move one line of code > up one space. My apologies. To be clear, there was no offense or insult intended here. I wasn't aware of bug 460090 (and neither was anybody else involved in the patch here). The past few months have been extremely hectic at Mozilla as we've tried to push Firefox 3.5 out the door and there has been a conscious effort to focus on bugs that directly block the release (i.e. blocking1.9.1+). Unfortunately, nobody noticed the severity of your bug and in the heat of the moment when the 0-day vulnerability hit the waves, that same nobody (of which I'm a part, not ducking responsibility) looked for a duplicate bug that might already contain a patch. Please accept my apology for the oversight and also please contribute to Jeff's thread on how to avoid this in the future.
(In reply to comment #23) > Passive-aggressive insinuations of bad faith generally don't win friends and > influence people. Jeff, he's frustrated with what happened here, so I think it's totally understandable he's being sarcastic.
(In reply to comment #24) > Unfortunately, nobody noticed the severity of your bug and in the heat of > the moment Honestly I am not trying to ask this in a rude way, but what process are you using that allows you to miss a bug marked with Critical importance (bug 460090)? There is talk of processes and documentation but here we had a bug with a patch, marked Critical. Shouldn't this show up at the top of at least one bugzilla list someone prioritizes on?
(In reply to comment #26) > (In reply to comment #24) > > Unfortunately, nobody noticed the severity of your bug and in the heat of > > the moment > > Honestly I am not trying to ask this in a rude way, but what process are you > using that allows you to miss a bug marked with Critical importance (bug > 460090)? That's a good question, I'm asking it too. I've been heads-down in TraceMonkey work, having long ago retired from release driving or bugzilla querying to find and triage important bugs. But I'm involved, on the hook even as the technical buck-stopper in mozilla.org. > There is talk of processes and documentation but here we had a bug > with a patch, marked Critical. Shouldn't this show up at the top of at least > one bugzilla list someone prioritizes on? That the bug was marked wanted1.9.1 instead of blocking1.9.1 when it involves a FreeMemoryRead was a mistake too. And of course that the bug was forgotten even by sicking, who apologized in bug 460090 comment 19 and obviously regrets the error, was poor. We've had bugs "get lost" but you're right that severity critical should be monitored and reviewed. It must not be debased. Processes and documentation won't solve every problem, of course, and they have their own overheads, as well as tending to be backward-looking. But they can absolutely help prevent a recurrence of what went wrong this time, in our world in a non-bureaucratic, personal-accountability fashion. We have people other than sicking in the Mozilla community who are on the spot here, as noted above (i.e., including me). I hope to hear from others who may have seen this bug and misjudged it, in the mozilla.governance thread if not here, so that your entirely reasonable questions are answered satisfactorily. Well, answered anyway. There may be little satisfaction, except in working to ensure that this doesn't happen again. Not looking to lay blame, rather (ideally) to fix the bug diagnosis, severity judging, and bug-marking/querying problems that probably exist here. /be
Verified fixed based on Tinderbox results on trunk and 1.9.1.
The crash with the POC doesn't repro in Thunderbird 22.214.171.124 (or 22pre) or Seamonkey nightly based on 126.96.36.199.
You need to log in before you can comment on or make changes to this bug.