Closed Bug 475185 Opened 11 years ago Closed 11 years ago

Crash [@ js_ValueToString] calling DOM setter function directly with no arguments

Categories

(Core :: XPConnect, defect, P1, critical)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9.1b3

People

(Reporter: jruderman, Assigned: mrbkap)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [sg:critical?])

Crash Data

Attachments

(3 files)

Attached file testcase
Crashes trying to dereference 0xdadadad8.
Flags: blocking1.9.1?
Whiteboard: [sg:critical?]
Attached patch FixSplinter Review
I always forget that fast natives don't enforce minargs.
Assignee: nobody → mrbkap
Status: NEW → ASSIGNED
Attachment #358639 - Flags: superreview?(brendan)
Attachment #358639 - Flags: review?(jorendorff)
OS: Mac OS X → All
Hardware: x86 → All
Comment on attachment 358639 [details] [diff] [review]
Fix

That's why I initially made JSFastNatives enforce minargs -- phear of phorgetting (myself or anyone, really).

No good way to automate assertion-based checking, alas. Maybe a static analysis?

/be
Attachment #358639 - Flags: superreview?(brendan) → superreview+
We need this for beta3 if we want to take the fix for bug 462428, which we do.
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P1
Target Milestone: --- → mozilla1.9.1b3
Comment on attachment 358639 [details] [diff] [review]
Fix

totally
Attachment #358639 - Flags: review?(jorendorff) → review+
Attached patch crashtestSplinter Review
Here's an automated test for whenever we check in automated tests for security bugs.
Flags: in-testsuite?
http://hg.mozilla.org/mozilla-central/rev/0ca5fbbf83dc
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Surely it's not necessary for a bug that was only in place for a bare handful of days?  One thing if it made it into a release (full, alpha, beta, RC), at which point I think you wait through the first fixed release before adding, but a run-of-the-mill nightly is a completely different thing.
You don't think it's necessary to check in a regression test for a bug that we only had briefly?  I don't think it matters whether we caught it quickly the first time; we don't want to have to worry about *not* catching it if we regress it later on branch or trunk.
That was in response to comment 5's "for whenever we check in automated tests for security bugs", which I was arguing should be immediately (or at best no longer than a week later) for a bug present for so short a time in nightlies only.
Ah, sorry, didn't understand the antecedent of "it".  Agreed, get it landed if we confirm that it doesn't affect any releases.
Regression test is ok, but it'll probably never fail again -- whereas there could be other fast natives that fall into the no-minargs trap -- or already have. Thus the static analysis hope. Jason, any thoughts?

/be
The crashtest is checked in (http://hg.mozilla.org/mozilla-central/rev/d936f7070bd5).
Flags: in-testsuite? → in-testsuite+
Group: core-security
verified fixed on the 1.9.1 branch using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090211 Shiretoko/3.1b3pre and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090211 Shiretoko/3.1b3pre. I verified using the testcase in the bug.
Flags: wanted1.9.0.x-
Crash Signature: [@ js_ValueToString]
You need to log in before you can comment on or make changes to this bug.