429239 - js.c trap() lets me corrupt a function by putting a trap at a bad offset

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

js> function a(n) { return n * n }
js> dis(a)
main:
00000:  getarg 0
00003:  getarg 0
00006:  mul
00007:  return
00008:  stop

Source notes:
js> trap(a, 2, "print('z')")
js> dis(a)
main:
00000:  getarg 83
00003:  getarg 0
00006:  mul
00007:  return
00008:  stop

Source notes:
js> a(2)  
Assertion failure: slot < fp->fun->nargs, at jsinterp.c:5411


83 is JSOP_TRAP.  I think trap() should complain that offset 2 is not an opcode instead of corrupting the function.  That information is clearly available, since dis() knows.

I think this bug makes it impossible to fuzz traps, since dis() information is not available to scripts (bug 396512).

Comment 1

[Brian Crowder]

If you could get dis() to give you back a string for the disassembly results, you could pick a valid trap opcode from the list at random.  I think that's the best way to address this bug.

Comment 2

[Brian Crowder]

This is really an example of poor API usage by the js shell, but it can be overcome with the help of my dis() patch from another bug, so I am going to INVA this one.

Comment 3

[Jesse Ruderman]

This could be a security issue for people who intentionally expose the shell to untrusted code.  Is the shell explicitly "not for that"?

Comment 4

[Atul Varma [:atul]]

As a workaround, would it be enough to just do "trap = null" before running any untrusted code?  I was planning on doing the same thing with load(), since I don't want it to be possible for the code to access the outside filesystem in any way, but I'm not sure if there's some other way to recover references to these functions.

Comment 5

[Jesse Ruderman]

You can hide all the shell builtins other than "print", while keeping things like eval and Math, with this code:

let (p=print) { clear(this); print=p; }

Dunno how foolproof this is.

Comment 6

[Jesse Ruderman]

This is being fixed in bug 476073.

Comment 7

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

function a(n) { return n * n }
trap(a, 2, "print('z')")
a(2)

is the testcase from comment #0 (re-posting to make copying/pasting from the shell easier), and confirming to still assert in latest TM tip and 1.9.0.x branch.

Comment 8

[Jesse Ruderman]

I wonder if removing JSOP_TRAP (bug 707454) took care of this.

Comment 9

[Brian Hackett [Laid off!]]

Yeah, trap() on an invalid bytecode offset should just be ignored now.