221526 - JS Script.thaw is a security hole

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Brendan Eich [:brendan]]

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]]

Attachment: quick and safe #ifdef fix

This is in the 1.5 branch already.

/be

Comment 2

[Brendan Eich [:brendan]]

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]]

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]]

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]]

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]]

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]]

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]]

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]

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]]

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]]

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)]

Now open as:

http://cve.mitre.org/cgi-bin/cvename.cgi?name=CAN-2003-0791

Comment 13

[Brendan Eich [:brendan]]

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)]

Opening.

Comment 15

[Brendan Eich [:brendan]]

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]

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]]

scole: I did that. 
http://lxr.mozilla.org/mozilla/search?string=JS_HAS_XDR_FREEZE_THAW shows it.

/be

Comment 18

[Steven Cole]

>sigh<.   That's what I get for not watching checkins as closely as I once did.
 Sorry for the spam, all...

--scole