JS Script.thaw is a security hole

RESOLVED FIXED in mozilla1.5final

Status

()

Core
JavaScript Engine
P1
blocker
RESOLVED FIXED
14 years ago
6 years ago

People

(Reporter: brendan, Assigned: brendan)

Tracking

({fixed1.4.1, fixed1.5})

Trunk
mozilla1.5final
fixed1.4.1, fixed1.5
Points:
---
Bug Flags:
blocking1.4.1 +
blocking1.5 +
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

14 years ago
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
(Assignee)

Comment 1

14 years ago
Created attachment 132831 [details] [diff] [review]
quick and safe #ifdef fix

This is in the 1.5 branch already.

/be
(Assignee)

Updated

14 years ago
Attachment #132831 - Flags: review?(shaver)
Attachment #132831 - Flags: approval1.5+
Attachment #132831 - Flags: approval1.4.2?
(Assignee)

Comment 2

14 years ago
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
Attachment #132831 - Flags: review?(shaver) → review+
(Assignee)

Updated

14 years ago
Attachment #132831 - Flags: approval1.4.2? → approval1.4.1?

Comment 3

14 years ago
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+
(Assignee)

Comment 4

14 years ago
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
Severity: normal → blocker
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla1.5final
(Assignee)

Comment 5

14 years ago
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
(Assignee)

Comment 6

14 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Flags: blocking1.5+
Flags: blocking1.4.1+
Keywords: fixed1.5
Resolution: --- → FIXED
(Assignee)

Updated

14 years ago
Keywords: fixed1.4.1
(Assignee)

Comment 7

14 years ago
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
(Assignee)

Comment 8

14 years ago
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

14 years ago
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
(Assignee)

Comment 10

14 years ago
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
(Assignee)

Comment 11

14 years ago
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
Now open as:

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

Comment 13

14 years ago
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

Updated

14 years ago
Blocks: 224532
Opening.
Group: security
(Assignee)

Comment 15

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

/be
Summary: JS Script.thaw is a *huge* security hole → JS Script.thaw is a security hole

Comment 16

14 years ago
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
(Assignee)

Comment 17

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

/be

Comment 18

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

--scole

Updated

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