Last Comment Bug 221526 - JS Script.thaw is a security hole
: JS Script.thaw is a security hole
Status: RESOLVED FIXED
: fixed1.4.1, fixed1.5
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: P1 blocker (vote)
: mozilla1.5final
Assigned To: Brendan Eich [:brendan]
: Phil Schwartau
Mentors:
Depends on:
Blocks: 224532
  Show dependency treegraph
 
Reported: 2003-10-07 15:27 PDT by Brendan Eich [:brendan]
Modified: 2011-08-05 22:35 PDT (History)
8 users (show)
brendan: blocking1.4.1+
brendan: blocking1.5+
bob: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
quick and safe #ifdef fix (2.99 KB, patch)
2003-10-07 15:27 PDT, Brendan Eich [:brendan]
brendan: review+
asa: approval1.4.1+
brendan: approval1.5+
Details | Diff | Splinter Review

Description Brendan Eich [:brendan] 2003-10-07 15:27:27 PDT
Script.thaw deserializes a script from a string.  You can then run the script. 
A savvy attacker who knows JS bytecode and engine internals, and the target
machine architecture, can call a native function!

Patch in a second.

/be
Comment 1 Brendan Eich [:brendan] 2003-10-07 15:27:56 PDT
Created attachment 132831 [details] [diff] [review]
quick and safe #ifdef fix

This is in the 1.5 branch already.

/be
Comment 2 Brendan Eich [:brendan] 2003-10-07 15:33:05 PDT
Comment on attachment 132831 [details] [diff] [review]
quick and safe #ifdef fix

Shaver had to catch a cab, but he gave r= over IRC.

/be
Comment 3 Asa Dotzler [:asa] 2003-10-07 15:37:08 PDT
Comment on attachment 132831 [details] [diff] [review]
quick and safe #ifdef fix

a=asa (on behalf of drivers) or checkin to 1.4.1
Comment 4 Brendan Eich [:brendan] 2003-10-07 15:43:05 PDT
This is so embarrassing, I think I'm going to go get drunk.  This hole has been
around since rev 3.2 of jsscript.c on 4/24/98 (Mozilla began, in general, with
rev 3.1 on 3/31/98).  The script freeze/thaw stuff was used by servers including
some that shaver was working on at the time.  We never included it in any
security review, for some reason.

Shaver, pour one for me at dinner tonight!

*Very heavy sigh*

/be
Comment 5 Brendan Eich [:brendan] 2003-10-07 15:44:50 PDT
Did I say "call a native function" in comment 0?  I meant "forge a native
function and then call any native code it likes."

See why I need a drink?

/be
Comment 6 Brendan Eich [:brendan] 2003-10-07 16:03:06 PDT
Ok, I've checked into the 1.4 and 1.5 branches as well as the trunk.  Marking
this fixed, but still waiting for respin update on 1.5.

/be
Comment 7 Brendan Eich [:brendan] 2003-10-07 16:05:49 PDT
So the fix excludes Script.prototype.freeze/thaw and Script.thaw from being
compiled into the MOZILLA_CLIENT embedding of SpiderMonkey.  Other browser
embedders need to be apprised.  In general, any web browser embedding should
follow our lead and defined MOZILLA_CLIENT, I think.

/be
Comment 8 Brendan Eich [:brendan] 2003-10-07 16:49:08 PDT
Cc'ing bryner, who is helping respin Firebird.

Cc'ing scole, whom I hereby swear to secrecy, and ask whether any planetweb
remnant exists that might need the patch.  Steve, do you recall whether
planetweb defined MOZILLA_CLIENT in its SpiderMonkey embedding?

/be
Comment 9 Steven Cole 2003-10-07 17:11:48 PDT
No, Planetweb never #defined MOZILLA_CLIENT.  (It's not clear that we want to,
given the other uses of MOZILLA_CLIENT in the engine.)

I'm not too worried about this, though.  Can the 'custom native function' be
present in the spoof string?  Or does it have to reside somewhere else in the
running binary image of the embedding?  If it has to live in the embedding
already, then I'm not worried at all. 

In any event, our embeddings don't (generally speaking) contain much that needs
to be protected, and the hardware architecture details are usually kept pretty
much under wraps.  (Not that that would stop a determined hacker, of course.)

I'll let management at PW know the story, though.  

[This needs more attention by embedders than hiding behind a security-bug will
give, though.  Should freeze and thaw be turned off unless explicitly turned on,
perhaps?  Then you don't really have to announce it, and the people who don't
pay lots of attention get the fix, too.]

--Steve
Comment 10 Brendan Eich [:brendan] 2003-10-07 17:25:18 PDT
Steve: the forged native function and its native code come from the string being
thawed into a script.  So this is a wide-open hole, if the attacker knows the JS
engine's internals and the target architecture.

Good point about turning freeze and thaw off in general.  We'll need a separate
JS_HAS_SCRIPT_FREEZE_THAW macro in jsconfig.h, defined to 0 for all versions
configured there.  I'll ponder that.

/be
Comment 11 Brendan Eich [:brendan] 2003-10-07 17:27:28 PDT
MOZILLA_CLIENT is actually good to define for all web browser embeddings of the
SpiderMonkey engine, because it turns off ECMA-standard things that are unsafe
on (Script.thaw), or incompatible (escape, unescape) with, the web.

/be
Comment 12 Christopher Blizzard (:blizzard) 2003-10-08 07:18:08 PDT
Now open as:

http://cve.mitre.org/cgi-bin/cvename.cgi?name=CAN-2003-0791
Comment 13 Brendan Eich [:brendan] 2003-10-08 08:09:52 PDT
I overstated the ease of exploit: there doesn't seem to be a way to serialize a
native function, or to corrupt a deserialized scripted function so that it
morphs into a native function.  That means any exploit will have to use buffer
overrun techniques (malloc size parameter wraparound).  Not easy, but possible.

I'm changing the comment above script_freeze like so:

- * we take them out of the Mozilla client altogether.  Script.thaw is a hole
- * wide enough that you could hack your own native function and call it!
+ * we take them out of the Mozilla client altogether.  Fortunately, there is
+ * no way to serialize a native function (see fun_xdrObject in jsfun.c).

I'll make this change to the trunk and all the branches, unless someone thinks I
should not make it in an old branch.  I'd like the branches to be consistent,
though.

/be
Comment 14 Christopher Aillon (sabbatical, not receiving bugmail) 2003-11-24 07:30:54 PST
Opening.
Comment 15 Brendan Eich [:brendan] 2003-12-01 11:22:35 PST
Per comment 13, this is not huge, just as big as any hard-to-exploit but
presumably exploitable malloc-size-param-overflow bug.

/be
Comment 16 Steven Cole 2003-12-01 16:59:13 PST
Since discussion is (sort of) active on this bug again: Is it appropriate to
think about/work on the jsconfig.h change mentioned in comment 10?  Or is a new
bug a better spot for that?

--scole
Comment 17 Brendan Eich [:brendan] 2003-12-01 17:42:44 PST
scole: I did that. 
http://lxr.mozilla.org/mozilla/search?string=JS_HAS_XDR_FREEZE_THAW shows it.

/be
Comment 18 Steven Cole 2003-12-01 19:22:24 PST
>sigh<.   That's what I get for not watching checkins as closely as I once did.
 Sorry for the spam, all...

--scole

Note You need to log in before you can comment on or make changes to this bug.