Closed Bug 429239 Opened 13 years ago Closed 8 years ago

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

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
critical

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: jruderman, Unassigned)

References

Details

(Keywords: assertion, testcase)

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).
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.
Depends on: 396512
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.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → INVALID
This could be a security issue for people who intentionally expose the shell to untrusted code.  Is the shell explicitly "not for that"?
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.
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.
Depends on: 476073
This is being fixed in bug 476073.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
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.
Depends on: 658805
I wonder if removing JSOP_TRAP (bug 707454) took care of this.
Yeah, trap() on an invalid bytecode offset should just be ignored now.
Status: REOPENED → RESOLVED
Closed: 12 years ago8 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.