390597 - watch point + eval-as-setter allows access to dead JSStackFrame

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[shutdown]

$ cat watch-point-dead-stack.txt
function exploit() {
  var obj = this, args = null;
  obj.__defineSetter__("evil", eval);
  obj.watch("evil", function() { return "args = arguments;"; });
  obj.evil = null;
  eval("print(args[0]);");
}
exploit();

$ gdb --eval run --args dbg.obj/js watch-point-dead-stack.txt
...
Program received signal SIGSEGV, Segmentation fault.
0x0043729b in JS_GetReservedSlot (cx=<incomplete type>, obj=0x1, index=0,
    vp=0xa4e2cc) at jsapi.c:3971
3971        clasp = OBJ_GET_CLASS(cx, obj);

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: FIx

Wow, I didn't know it was possible to create an arguments object for the pseudo-frame. It's getting to be more and more like a real frame every day!

Comment 2

[Brendan Eich [:brendan]]

Comment on attachment 275031 [details] [diff] [review]
FIx

Indeed. Need to be careful there isn't more like this (I don't know of any, just sayin').

/be

Comment 3

[Brendan Eich [:brendan]]

Comment on attachment 275031 [details] [diff] [review]
FIx

Sorry, a=me.

/be

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk, finally.

Comment 5

[Bob Clary [:bc] (inactive)]

Attachment: js1_5/extensions/regress-390597.js

I am sure this is a stoopid question, but why does the browser throw "EvalError: function eval must be called directly, and not by way of a function of another name" but the shell doesn't?

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Sounds like another XOW problem. Try using 'obj.eval' as the setter function?

Comment 7

[Bob Clary [:bc] (inactive)]

(In reply to comment #6)
> Sounds like another XOW problem. Try using 'obj.eval' as the setter function?
> 

that removed the EvalError. Bug?

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Yeah, it's a bug.

Comment 9

[Bob Clary [:bc] (inactive)]

bug 397071

Comment 10

[Bob Clary [:bc] (inactive)]

verified fixed 1.9.0 linux/mac*/win

this crashes in the browser 1.8.1. do we want it on the branch?

Comment 11

[Daniel Veditz [:dveditz]]

blake, brendan: does this patch work for the 1.8 branch or do we need a different fix (or none)? Can we land this in a day or two, or should we wait for 1.8.1.11?

Comment 12

[Samuel Sidler (old account; do not CC)]

If this patch applies to the 1.8 branch can you please request approval? Otherwise, please attach a branch patch.

(QA: To test, use the JS 'shell' bookmarklet.)

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: 1.8 branch patch

I had to hand merge because the watchpoint thread-safety fixes never went in on the 1.8 branch. The patch was trivial to merge.

Comment 14

[Daniel Veditz [:dveditz]]

Comment on attachment 294507 [details] [diff] [review]
1.8 branch patch

approved for 1.8.1.12, a=dveditz for release-drivers

Comment 15

[Blake Kaplan (:mrbkap) (inactive)]

Fixed on the 1.8 branch.

Comment 16

[Stephen Donner [:stephend] Not actively reading bugmail]

I crashed 2.0.0.11 by pasting (with much-cleaner linebreaks!):

function exploit() { try { var obj = this, args = null; obj.__defineSetter__("evil", eval); obj.watch("evil", function() { return "args = arguments;"; }); obj.evil = null; eval("print(args[0]);"); } catch(ex) { print('Caught ' + ex); } } exploit();

into Jesse's handy JS "shell" bookmarklet, on Windows XP.

With Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.12) Gecko/20080128 Firefox/2.0.0.12, I get |undefined|, and no crash.

Replacing fixed1.8.1.12 with verified1.8.1.12; special thanks to bc for the support/sanity-checking.

Comment 17

[Alexander Sack]

Comment on attachment 294507 [details] [diff] [review]
1.8 branch patch

a=asac for 1.8.0.15

(unmodified distro patch)

Comment 18

[Reed Loden [:reed]]

MOZILLA_1_8_0_BRANCH:

Checking in js/src/jsdbgapi.c;
/cvsroot/mozilla/js/src/jsdbgapi.c,v  <--  jsdbgapi.c
new revision: 3.56.2.1.4.13; previous revision: 3.56.2.1.4.12
done

Comment 19

[Bob Clary [:bc] (inactive)]

/cvsroot/mozilla/js/tests/js1_5/extensions/regress-390597.js,v  <--  regress-390597.js
initial revision: 1.1