374260 - enumerating over the java, netscape or Packages objects results in crash

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[lsg-mtso]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.2) Gecko/20070219 Firefox/2.0.0.2
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a3pre) Gecko/20070316 Minefield/3.0a3pre

The following causes a crash in the Java Embedding Plugin:
<script>
alert("prepare for crash");
for(x in Packages) {
  document.write(x);
}
</script>

No clue if this belongs to the DOM folks, jscript proper, or plugins...since it seems to be in the plugin itself, plugins win.

Of possible concern is the presence of ascii text as the crash location -- address is the string 'try' backwards -- suggesting the source of the value is an unitialized variable, heap value, or something that may be alterable by an attacker.


Reproducible: Always

Steps to Reproduce:
1.run attached html file
Actual Results:  
crash

Expected Results:  
contents of referenced objects

Comment 1

[lsg-mtso]

Attachment: backtrace for java embedding plugin crash

Comment 2

[lsg-mtso]

Attachment: repro html

Comment 3

[Martijn Wargers (dead)]

This is worksforme with current trunk and 1.8.1 branch build on windows.

Comment 4

[lsg-mtso]

Sorry -- browser crashed when I was filling this out originally. ;)  I can confirm this does *not* happen under Win32 -- I only see it on OSX.

Comment 5

[Jesse Ruderman]

The testcase is similar to that of bug 344804 and bug 349964.

Steven, can you reproduce the crash?

Comment 6

[Steven Michaud [:smichaud] (Retired)]

I can reproduce this crash (and another very different one) with
Minefield, but not with any of the non-trunk Mozilla.org browsers that
I tried (Firefox 2.0.0.3, Camino 1.0.4, Seamonkey 1.1.1).

I'm convinced that this is a Mozilla.org bug/design-flaw, and that
it's related to bug 347598.  I'm currently trying to track the bug
down ... which may take a few days.  I'll comment again when I have
something more definite to say.

Comment 7

[Steven Michaud [:smichaud] (Retired)]

Attachment: Non-corrupt stack trace (from non-stripped JEP)

After some digging around, I've found that (at least) three bugs are involved
here -- one of them is a JEP bug, and the other two are browser bugs (I'll
talk about the browser bugs in later messages).

The JEP bug is a crash that happens when the JEP iterates over all the
browser's windows (in [JEPController hookExistingObjects] or [JEPController
allSubviews] (called from hookExistingObjects)), and the last NSWindow's
contentView turns out to be invalid (which triggers the crash).  I'm not sure
why the contentView is invalid (it may be an Apple bug, or conceivably a
browser bug).  But it's pretty clear that the NSWindow in question is the
alert box (the one that says "prepare for crash"), which has just been
dismissed (by clicking on its "OK" button).  The crash doesn't happen when I
comment out 'alert("prepare for crash");'  Though the partially invalid alert
window is still in NSApp's window list, it's no longer visible.  So I'll work
around this problem by ignoring NSWindows in the window list that aren't
visible.

The stack trace that lsg-mtso posted in comment #1 is slightly corrupt (there
can't have been any call to ShowClassInfo) -- possibly because bundled copies
of the JEP have their debug symbols stripped.  The trace I'm posting here is
more accurate (it was made using an unstripped copy of JEP 0.9.6).

Most of my crashes don't seem to have any security implications.  But one of
them (from a stripped JEP) did also have "ascii text as the crash location":

  Exception:  EXC_BAD_ACCESS (0x0001)
  Codes:      KERN_INVALID_ADDRESS (0x0001) at 0x6e3d314e

Comment 8

[Steven Michaud [:smichaud] (Retired)]

Attachment: Oddball crash, possibly irrelevant

I mentioned in comment #6 that I also saw a second, very different crash.
Here's a stack trace (one of two nearly identical ones).  This is just for the
record -- I have no idea if it's relevant.

Comment 9

[Steven Michaud [:smichaud] (Retired)]

Even if you comment out 'alert("prepare for crash");' (to avoid the crash),
you still see wierdness:

No list of packages is displayed.  In the Error Console you see "Error: too
much recursion".  And in the Java Console you see several hundred lines
starting as follows (which shows that this is indeed a recursion bug):

java.lang.NoClassDefFoundError: __iterator__
  at netscape.oji.JNIRunnable.run(Native Method)
  at netscape.oji.LiveConnectProxy.run(LiveConnectProxy.java:48)
java.lang.NoSuchMethodError: createClassLoader
  at netscape.oji.JNIRunnable.run(Native Method)
  at netscape.oji.LiveConnectProxy.run(LiveConnectProxy.java:48)
java.lang.NoClassDefFoundError: __iterator__/toSource
  at netscape.oji.JNIRunnable.run(Native Method)
  at netscape.oji.LiveConnectProxy.run(LiveConnectProxy.java:48)
java.lang.NoSuchMethodError: createClassLoader
  at netscape.oji.JNIRunnable.run(Native Method)
  at netscape.oji.LiveConnectProxy.run(LiveConnectProxy.java:48)
java.lang.NoClassDefFoundError: __iterator__/toSource/toSource
  at netscape.oji.JNIRunnable.run(Native Method)
  at netscape.oji.LiveConnectProxy.run(LiveConnectProxy.java:48)
java.lang.NoSuchMethodError: createClassLoader
  at netscape.oji.JNIRunnable.run(Native Method)
  at netscape.oji.LiveConnectProxy.run(LiveConnectProxy.java:48)
java.lang.NoClassDefFoundError: __iterator__/toSource/toSource/toSource
  at netscape.oji.JNIRunnable.run(Native Method)
  at netscape.oji.LiveConnectProxy.run(LiveConnectProxy.java:48)
...

The "java.lang.NoSuchMethodError: createClassLoader" lines are irrelevant,
benign, and the symptom of yet another browser problem (one I won't discuss
here).  The ones I'm interested in are those that include "__iterator__".

This "__iterator__" operator is used internally to implement iteration (a new
feature of JavaScript 1.7) -- needed to interpret the lines

for(x in Packages) {
  document.write(Packages[x]);
}

But the __iterator__ operator _isn't_ supported in Java.  So trying to use it
in Java is a bug.  (This is the first browser bug.)

The second browser bug is what causes the recursion.  It's in the
js_ReportIsNotFunction() function in js/src/jsfun.c.  When the JavaScript
interpreter tries to use the __iterator__ operator in Java it gets a
NoClassDefFoundError and calls js_ReportIsNotFunction().  Since this is a case
of a bad iterator, js_ValueToPrintableSource() gets called (under "if (flags &
J2VF_ITERATOR)") -- which tries to call the bad iterator's "toSource" method.
But Java doesn't have an __iterator__/toSource class or method -- so
js_ReportIsNotFunction() gets called again (recursively), and the same thing
happens again, and keeps happening until some kind of internal recursion limit
is reached.

I plan to open bugs on each of these problems.  I'll use a variant of
lsg-msto's test case, without the call to alert() (which should avoid spilling
any security-sensitive beans).

By the way, the code in lsg-msto's test case is partially invalid.  If
Mozilla.org's JavaScript interpreter could iterate over an array of Java
objects, and lsg-msto's code worked properly, it would just display the values
of 'x' -- not the names of the Java objects in the array.  To display the
names of the Java objects you'd need something like this:

for(x in Packages) {
  document.write(Packages[x]);
}

Comment 10

[Steven Michaud [:smichaud] (Retired)]

> I'm convinced that this is a Mozilla.org bug/design-flaw, and that it's
> related to bug 347598.

I was wrong -- this bug is completely unrelated to bug 347598.  But that's
also a browser bug, and I've managed to find a fix for it.  I'll report my fix
at bug 347598.

Comment 11

[Steven Michaud [:smichaud] (Retired)]

I've just released a new version (0.9.6.1) of the Java Embedding
Plugin that fixes the JEP crash that I described in comment #7.  For
more information see bug 376395.

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

This looks like a nullpointer deref per comment 7, so not blocking. However it would be great if we can get this fixed.

Steven: any chance you could come up with a fix for 1.9?

Comment 13

[Steven Michaud [:smichaud] (Retired)]

> This looks like a nullpointer deref

Actually the crash bug is something more serious -- it's caused by the
last window in the window list (what's returned by [NSApp windows])
having an invalid contentView -- one that's already been deleted.  As
I say in comment #7, this might be a browser bug (though it could also
be an Apple bug).

In any case (as I say in comment #11), this crash bug has already been
worked around in the current version (now 0.9.6.2) of the Java
Embedding Plugin, which has been landed on the trunk and both branches
(and is set to appear in Firefox 2.0.0.4 and 1.5.0.12).

> Steven: any chance you could come up with a fix for 1.9?

The other problems (what I talk about in comment #9) haven't yet been
fixed.  Yes, I'll probably be able to fix those by 1.9, but I'm not
going to be able to work on them right away.

Comment 14

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

So from the sounds of it this isn't exploitable any more? Is that right? If not, please renominate the bug for 1.9 blocking.

It would be great to get separate bugs filed for the problems that remain (which you mentioned in comment 9). As it stands this bug seems to cover many problems which is always confusing.

Comment 15

[Steven Michaud [:smichaud] (Retired)]

> So from the sounds of it this isn't exploitable any more?

I doubt that the crash bug was ever exploitable (we only have very
slight evidence of exploitability, and that comes from stacks created
using stripped binaries).  But yes, now that the new JEP has worked
around it, it's not even potentially exploitable (at least via the
JEP).

It's still possible that the invalid/deleted contentView problem is a
browser bug (and not an Apple bug).  But (as far as I know) crashes
that result from continuing to use a deleted object usually aren't
exploitable.  So this problem isn't an urgent one.

> It would be great to get separate bugs filed for the problems that
> remain

I agree.  But I don't have a whole lot of time to work on this, so it
won't happen right away.

I'd say that the two problems mentioned in comment #9 belong in their
own bugs.  And if it can be shown that the deleted contentView problem
is a browser bug, that should also get its own, separate, bug report.
Likewise if the deleted contentView problem is an Apple bug, but it's
possible that the browser might stumble across it (e.g. if it ever
iterates through the [NSApp windows] window list).

Comment 16

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Calling virtual methods on a deleted object is actually exploitable most of the time (though i'd rather have the discussion on how elsewhere). Simply using members might not be depending on what the members are used for.

Comment 17

[Steven Michaud [:smichaud] (Retired)]

OK.  I'd guess the crucial difference is that virtual methods use a
vtable and others don't.

Though there are different "flavors" of Objective C (C and C++), I
don't know whether the Objective C runtime (which is what crashes in
this case) ever uses virtual methods.  I suspect not (Objective C is
quite old, and is part of the Next legacy).

I will try to find time in the next week to find out whether the
contentView problem is a browser bug, or whether there's any code in
the browser that iterates through the window list and touches those
windows' contentViews.

But if the crash bug is _really_ urgent, it's probably best to get
someone else to work on it.

Comment 18

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Ok, reassigning to Colin to at least figure out if this is exploitable or not, though I suspect it is.

Comment 19

[Colin Barrett [:cbarrett]]

I don't believe so, although, I couldn't definitively say one way or the other. 

A method is invoked, internally, by calling objc_msgSend which takes an object, a SEL (const char * name of the method), and then variable arguments. What happens then is that the SEL is looked up in a table and, based upon the object's class, and transformed into an IMP, which is a function pointer like so id (*IMP)(id, SEL, ...). All ObjC methods are actually functions that take two additional arguments, id self and SEL _cmd. That's, in a nutshell, how the runtime works.

I'm not sure how you would exploit that.

Comment 20

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to comment #19)
> the SEL is looked up in a table and, based upon the
> object's class, and transformed into an IMP, which is a function pointer like
> so id (*IMP)(id, SEL, ...).

The question is, how does the runtime determine the object's class and find the table from |char *| -> IMP? In C++, this is done by having each object carry a vtable around with it (or really, a vtable ptr). When a virtual method is called, the generated code looks up the method address from a fixed offset into the vtable. If the object is deleted, then the vtable ptr will point to invalid memory, and the resulting function address will be bogus, but we'll still treat that address like a real function.

Comment 21

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I talked with Colin about this and it sounds like this is exploitable. It might be harder to do it, and you'll get a lower percentage of attempts succeeding, but it seems very doable still.

Comment 22

[Steven Michaud [:smichaud] (Retired)]

As far as I know, an object's vtable is actually in the object, at a
known (or easily knowable) location, but the SEL -> IMP table is
elsewhere (there's a single table maintained by the Objective C
Runtime).

So I'm going to keep making trouble, and say that I still don't think
this crash bug is exploitable :-)

Comment 23

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Ok, so here's the rundown:

In ObjC all objects contain an isa pointer. This pointer points to the class description containing the class name, what the class inherits, the functions contained in the class, etc. For each function there's a name as a string and a pointer to where the code for the function lives. All objects of the same class have isa pointers pointing to the same class description.

Function dispatch gets the isa pointer. It then looks up the requested function name in the list of functions for the class, gets the pointer for the implementation of that function and calls it.

Here is how you exploit a call to function "myFunc" on a deleted object.

1. Fill memory with |nop| instructions tailed with evil code that hacks your
   machine. Create several of these chunks all over memory.
2. Spread fake class description tables all over memory. This table should at
   least contain a function with the name "myFunc". The implementation pointer
   is a random address in memory.
3. Make the code make a call to the "myFunc" function on a deleted object
4. The ObjC dispatch code will read the isa pointer on the deleted object and
   get a random value back since it's reading deleted memory.
5. Chances are that the isa pointer will point to a fake class description table
   since they are all over the heap.
6. The ObjC dispatch code will dispatch to the random address in the fake class
   description table.
7. Chances are that this will end up at one of the nop-runs ending with the evil
   code.

This is more or less exactly the same as a standard exploit C++ calling a virtual function on a deleted object which has been shown to work with high success rate. The only difference is that you need to go through the fake class description table, but that only reduces your chances of success, not eliminates it.

Comment 24

[Steven Michaud [:smichaud] (Retired)]

I think this scenario is very much less likely than one that uses a
deleted object containing a vtable -- there's a much greater chance
that a virtual function's vtable pointer (containing random data)
points to a nop instruction (leading up to evil code) than that a
deleted Objective C's isa pointer points to the beginning of an evil
class description table (a class description table is 40 bytes, and a
nop is (I think) just one byte).  But I concede that it's not
impossible.

Thanks for your explanation -- I've learned something from it (and
from the rest of this discussion).

Comment 25

[Steven Michaud [:smichaud] (Retired)]

I've found that the crash bug is a browser bug, and have spun it off in bug
381087.

Comment 26

[Steven Michaud [:smichaud] (Retired)]

Jesse just added the keyword "testcase" to this bug.  I assume this
means that a testcase can be found here (which is true).  But I need
to say how the testcase should be used for bug 381087 (which is where
the "deleted contentView" part of this bug has been spun off).

(By the way, the reason I'm talking about this here rather than in bug
381087 is that this bug is potentially security-sensitive.)

To use this bug's testcase (attachment 258840 [details]) on bug 381087, you need
to make sure your trunk build's bundled version of the JEP is 0.9.6 or
older (later versions contain a workaround for bug 381087).  The
simplest way to do this is to download JEP 0.9.6 (from
http://javaplugin.sourceforge.net/) and copy that distro's
JavaEmbeddingPlugin.bundle and MRJPlugin.plugin over the existing
copies of these bundles in your trunk build's Contents/MacOS/plugins
directory.

Trunk nightlies built before the fix for bug 373122 was landed should
crash with a trace that looks like the "non-currupt stack trace"
(attachment 259607 [details]).  Trunk nightlies built after that fix was landed
shouldn't crash.  (The fix for bug 373122 also fixes bug 381087.)

Comment 27

[Jesse Ruderman]

Steven says the sg:critical crash is fixed (I guess that refers to comment 11).  I'm resolving this as fixed to reduce confusion.

Steven, can you file bugs on the remaining problems, even if you're not planning to fix them right away?

Comment 28

[Steven Michaud [:smichaud] (Retired)]

> Steven, can you file bugs on the remaining problems, even if you're
> not planning to fix them right away?

Yes, I'll do that later this week or early next week.

Comment 29

[Steven Michaud [:smichaud] (Retired)]

> Steven says the sg:critical crash is fixed (I guess that refers to
> comment 11).

No.  I'm referring to comment #25 (and comment #26).

Comment 30

[Daniel Veditz [:dveditz]]

qawanted: Does this affect the 1.8 branch as well? We use the same (or nearly) JEP and haven't really touched Java/OJI/Liveconnect so I'd expect so.

Comment 31

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #30)

If you mean the crash bug that was spun off into bug 381087, and which
triggered crashes in old versions of the JEP (0.9.6 and older) the
answer is "no".

(By the way, the JEP version in all current Mozilla.org browsers is
now 0.9.6.2.)

Comment 32

[Steven Michaud [:smichaud] (Retired)]

The other two problems are also browser bugs (in LiveConnect, see
comment #9), but aren't security related.  So none of the three
different bugs reported here is caused by the JEP, though the crash
bug could be triggered by older versions of the JEP.

Comment 33

[Steven Michaud [:smichaud] (Retired)]

I'm pretty sure that the "two other bugs" reported in comment #3 _do_
effect the 1.8 branch (and all other current branches), but I haven't
tested to be sure.

Comment 34

[Samuel Sidler (old account; do not CC)]

Steven, have you had time to test? Are those bugs filed separately? If this bug, on its own, doesn't affect the branch, can you add "[post 1.8-branch]" to the status whiteboard? (If there are other, unfiled security bugs that affect the branch, please file them and request blocking.)

Comment 35

[Steven Michaud [:smichaud] (Retired)]

The only unfinished business here is opening separate bugs for the
"two other bugs" mentioned in comment #3.  I hope to file them by the
end of this week.  (Neither of these bugs is a security bug, and
neither of them is urgent, or likely to be attended to anytime soon
even after I file them.)

> If this bug, on its own, doesn't affect the branch, can you add
> "[post 1.8-branch]" to the status whiteboard?

"This bug" is a mess.  The original report, while technically correct,
was totally misleading.  I did eventually find the real problem, which
I spun off into bug 381087 to avoid confusion.

I think this was the correct thing to do ... but it hasn't avoided
confusion :-)

So (for some purposes at least) I've been treating "this bug" as a dup
of bug 381087.  That's why Jesse (on my recommendation) marked it
FIXED.  So in the same spirit I'll add "[post 1.8-branch]" to the
status whiteboard.  While I'm at it I'll remove qawanted and testcase
from the keywords.

> If there are other, unfiled security bugs that affect the branch,
> please file them and request blocking.

There aren't any.

Comment 36

[Steven Michaud [:smichaud] (Retired)]

> The original report, while technically correct, was totally
> misleading.

I skimmed the bug yet again and found part of the reason why people
have gotten so confused about this bug -- the "user agent" string in
comment #0 refers to Firefox 2.0.0.2.

But comment #0's "build identifier" refers to a Minefield nightly.
And the two relevant crash logs (attachment 258839 [details] and attachment
259607) were both made using Minefield.

Comment 37

[Steven Michaud [:smichaud] (Retired)]

> "two other bugs" mentioned in comment #3

Sorry.  This should have been:

> "two other bugs" mentioned in comment #9

Comment 38

[Steven Michaud [:smichaud] (Retired)]

I've opened bugs for the two problems I mentioned in comment #9:

"LiveConnect mistakenly tries to use a Java __iterator__ method"
bug 389751

"js_ReportIsNotFunction can go into an infinite loop"
bug 389863

I also opened bug 389597 ("Use Java Package.getPackage() in JavaScript
JavaPackage_resolve()"), which contains a patch that fixes bug 347598
(mentioned several times above) and stops the "infinite loop" from
happening with this bug's test case.