My changes for bug 412296 removed the check for Date class in the implementation of Date.prototype.setTime method. This allows to overwrite slots with jsNaN value in arbitrary object. The following test case demonstrates the problem: ~/m/tm/js/src> cat ~/s/x.js var x = new Function('return 10'); Date.prototype.setTime.call(x); print(x()); ~/m/tm/js/src> ~/build/js32.tm.dbg/js ~/s/x.js Assertion failure: STOBJ_GET_CLASS(obj) == &js_DateClass, at /home/igor/m/tm/js/src/jsdate.cpp:1227 Trace/breakpoint trap

The fix is rather non-minimal but I wanted to ensure that I have not missed any more cases like this bug. The extra change removes double-boxing of time as jsdouble jsval in SetUTCTime callers and removal of date_constructor as the new SetUTCTime can serve its function just as well.

Comment on attachment 408034 [details] [diff] [review] fix v1 >@@ -1707,32 +1692,36 @@ date_makeDate(JSContext *cx, uintN maxar > uintN i; > jsdouble lorutime; /* local or UTC version of *date */ > jsdouble args[3], *argp, *stop; > jsdouble year, month, day; > jsdouble result; > > obj = JS_THIS_OBJECT(cx, vp); > if (!GetUTCTime(cx, obj, vp, &result)) >- return JS_FALSE; >+ return false; > > /* see complaint about ECMA in date_MakeTime */ >- if (argc == 0) >- return SetDateToNaN(cx, vp); >+ if (argc == 0) { >+ SetDateToNaN(cx, obj, NULL); No need for trailing NULL. >@@ -1813,46 +1798,46 @@ date_setYear(JSContext *cx, uintN argc, [snip] >+ if (!JSDOUBLE_IS_FINITE(year)) { >+ SetDateToNaN(cx, obj, vp); >+ return true; >+ } > > year = js_DoubleToInteger(year); > > if (!JSDOUBLE_IS_FINITE(result)) { > t = +0.0; > } else { > t = LocalTime(result); > } While you are here, may as well remove overbracing or even switch to y = ... ? ... : ...; >@@ -2586,17 +2525,17 @@ js_DateSetYear(JSContext *cx, JSObject * > MonthFromTime(local), > DateFromTime(local), > HourFromTime(local), > MinFromTime(local), > SecFromTime(local), > msFromTime(local)); > > /* SetUTCTime also invalidates local time cache. */ >- SetUTCTime(cx, obj, NULL, UTC(local)); >+ SetUTCTime(cx, obj, UTC(local)); Lame that these ancient (Netscape 2 era, for the server-side JS in "LiveWire") Friend APIs are void. File a followup bug? Good catch on the double boxing. /be

The new patch addresses the comment 2.

Regarding the severity of this bug: it allows, for example, to overwrite the slots that stores array's length and count with a pointer to jsdouble containing NaN. Typically such pointer, when reinterpreted, would represent relatively big number that significantly exceeds array bounds. So the bug allows to bypass array bound checks and read-write pretty much arbitrary value after malloced memory containing array's elements.

The test case checks that all Date.prototype methods throw a TypeError when called with an incompatible this object.

this caused (I think) Bug 524671

(In reply to comment #7) > this caused (I think) Bug 524671 (It didn't, per that bug.) Is this patch ready for 1.9.1? Can you please request approval if so? Code freeze is scheduled for November 10 at 11:59pm.

See comment 5 regarding the danger of this bug.

This backport to 1.9.1 includes the fix for the regression from the bug 527027.

This is a proof that a backport was trivial one and patch for 1.9.2 failed to apply to 1.9.1 due to code movements.

Comment on attachment 411194 [details] [diff] [review] fix for 1.9.1 Approved for 1.9.1.6, a=dveditz for release-drivers

I sometimes pipe a plain diff of two patches into grep -v '^[<>] ' to show the essential differences. /be

Can we get this landed before code freeze tonight at 11:59pm PST?

(In reply to comment #16) > Can we get this landed before code freeze tonight at 11:59pm PST? The bug has already landed on 1.9.1, see comment 15.

Yeah, sorry. My query was messed up. :-/

Comment on attachment 411194 [details] [diff] [review] fix for 1.9.1 Ah ha! My query wasn't messed up. Dan approved this for 1.9.0 instead of 1.9.1. Switching approval flags to clean up queries.

These bugs landed after b4 was cut. Moving flag out.

can we get some eyes on Bug 535550 to figure out if that new crash in 3.5.6 is a possible regression from this fix?