Closed Bug 221526 Opened 21 years ago Closed 21 years ago

JS Script.thaw is a security hole


(Core :: JavaScript Engine, defect, P1)






(Reporter: brendan, Assigned: brendan)



(Keywords: fixed1.4.1, fixed1.5)


(1 file)

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.

This is in the 1.5 branch already.

Attachment #132831 - Flags: review?(shaver)
Attachment #132831 - Flags: approval1.5+
Attachment #132831 - Flags: approval1.4.2?
Comment on attachment 132831 [details] [diff] [review]
quick and safe #ifdef fix

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

Attachment #132831 - Flags: review?(shaver) → review+
Attachment #132831 - Flags: approval1.4.2? → approval1.4.1?
Comment on attachment 132831 [details] [diff] [review]
quick and safe #ifdef fix

a=asa (on behalf of drivers) or checkin to 1.4.1
Attachment #132831 - Flags: approval1.4.1? → approval1.4.1+
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*

Severity: normal → blocker
Priority: -- → P1
Target Milestone: --- → mozilla1.5final
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?

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.

Closed: 21 years ago
Flags: blocking1.5+
Flags: blocking1.4.1+
Keywords: fixed1.5
Resolution: --- → FIXED
Keywords: fixed1.4.1
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.

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?

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: 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.

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.

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,

Blocks: 224532
Per comment 13, this is not huge, just as big as any hard-to-exploit but
presumably exploitable malloc-size-param-overflow bug.

Summary: JS Script.thaw is a *huge* security hole → JS Script.thaw is a security hole
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?

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

Flags: testcase-
You need to log in before you can comment on or make changes to this bug.