Another mutable __proto__ bug: -- function f() {} function g(o) { o.__proto__ = function() {}; f = Object.prototype; Object.seal(o); } g(f); g(f); -- Asserts on JM branch (works on TM branch): $ ./js test.js Assertion failure: newShape->slot == lastProperty()->slot, at ../jsobjinlines.h:795

Hmm. This is a TM bug: the assignment to __proto__ is a SETMETHOD, the function(){} doesn't get cloned on the first call to g and has properties added to it directly when Object.seal snapshots it. My understanding of js_CloneFunctionObject is that it should only be used on functions that are empty, i.e. we shouldn't be putting properties on a function and then cloning it later. That isn't asserted anywhere by TM (why this testcase works in the TM branch), but JM has a weakened form of such an assertion which is tripping here (we copy the type/shape from the original function to the cloned function). This testcase exposes the problem in a TM shell: function foo() { var x = {}; x.__proto__ = function() { return 0 } return x; } var a = foo(); var b = foo(); print(a.__proto__ === b.__proto__); d8/jsc: false js: true

So obj_getProto would have to invoke the method barrier, basically, to eliminate this.

Testcase from bug 648837 comment 2: a = <x><y/></x>; a.function::__proto__ = null; -a This testcase seems to also be related. Tested on JM rev 17cbc8fed578 with -n, 64-bit shell in 10.6.

I'm not so sure about it being related, myself -- it doesn't involve function cloning, or a needed __proto__ method barrier, at all. But I think bug 646129's patch would likely fix it.

eval("\ for(let y;;y,x.E=function(){}) {\ function x(){}(<x/>)\ }\ ") is a testcase that also asserts identically, but which crashes in a 32-bit opt shell on changeset 325744fbf7f0 without any CLI arguments at JSObject::setSlot. Snippet of stack: (gdb) bt #0 0x000e7235 in JSObject::setSlot () at jsobj.h:706 #1 0x000e7235 in JSObject::nativeSetSlot () at /Users/fuzz2/Desktop/jsfunfuzz-dbg-32-jm-68840-325744fbf7f0/patched/compilePath/js/src/jsobjinlines.h:1089 #2 0x000e7235 in js_NativeSet (cx=0x51a660, obj=0x6110e0, shape=0x617258, added=false, strict=false, vp=0xbfffe640) at /Users/fuzz2/Desktop/jsfunfuzz-dbg-32-jm-68840-325744fbf7f0/patched/compilePath/js/src/jsobj.cpp:5669 #3 0x000e7a02 in js_SetPropertyHelper (cx=0x51a660, obj=0x6110e0, id=3261648, defineHow=<value temporarily unavailable, due to optimizations>, vp=0xbfffe640, strict=0) at /Users/fuzz2/Desktop/jsfunfuzz-dbg-32-jm-68840-325744fbf7f0/patched/compilePath/js/src/jsobj.cpp:6174 #4 0x000a722a in js::Interpret (cx=0x51a660, entryFrame=0x10000b0, inlineCallCount=0, interpMode=js::JSINTERP_NORMAL) at /Users/fuzz2/Desktop/jsfunfuzz-dbg-32-jm-68840-325744fbf7f0/patched/compilePath/js/src/jsinterp.cpp:4439

(In reply to comment #6) > eval("\ > for(let y;;y,x.E=function(){}) {\ > function x(){}(<x/>)\ > }\ > ") Maybe dupe of bug 648837 too (also a bug in TM that bites JM).

Patch for the first testcase. Under SETMETHOD on __proto__, we took a path that called the shape's set() property without cloning the function first.

The testcase in comment 4 gives me a stack overflow on TM tip, JM tip and the named JM revision. Should be fixed by bug 646129, made a note in that bug.

Assigning to bhackett because he's clearly working on it. Making a hidden security bug based on comment in bug 648837 that this is similar. Did it turn out to be similar and potentially exploitable?

This is a similar case to bug 648837; this could cause semantic bugs on TM but not crashes, and is only a problem on the Jaegermonkey branch. Above patch landed to JM: http://hg.mozilla.org/projects/jaegermonkey/rev/94ae102189f0

Comment on attachment 538062 [details] [diff] [review] patch Review of attachment 538062 [details] [diff] [review]: -----------------------------------------------------------------