366082 - Possible Remote Code Execution from adobe reader (mem corruption)

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Daniel Veditz [:dveditz]]

The PDF vulnerability announcement by Stefano Di Paola -- http://www.wisec.it/vulns.php?page=9 -- made four claims, of which most attention has focused on claim 2 (XSS execution in Firefox) and which is fixed in Adobe Reader 8.0.

This bug is about claim 3, in case it is *not* an adobe problem but a general plugin one.

<blockquote>
   3. Possible Remote Code Execution

   There is also a possible Remote code Execution by leveraging a memory
   corruption inside Firefox by supplying the following request:

   http://site.com/file.pdf#FDF=javascript:document.write('jjjjj...');

   It's possible to cause a DoubleFree() error and to overwrite part of
   the Structural Exception Handler.

   Runtime vulnerability analisys
   The problem seems to be caused by a "Double MSVCRT.free()" executed
   by Acrobat plugin. The routine which cause Firefox to crash is located
   in the following call to NP_Shutdown().

   Elia Florio is credited for Runtime analysis and exploitation.
</blockquote>

When testing I usually see an alert from Firefox saying the plugin has performed an illegal operation and that I should restart Firefox. I've not seen a crash in NP_Shutdown() as described, but in other places (often like TB28066964).

I've seen crashes/hangs with strings as short as

http://www.mozilla.org/press/nytimes-firefox-final.pdf#blah=javascript:document.write("foopy");

and consistently with long strings (but not necessarily long URLs) like

http://www.mozilla.org/press/nytimes-firefox-final.pdf#blah=javascript:s='';for(i=0;i<10000;++i)s+='AAAAAAAAAA';document.write(s);

In the couple of cases I caught in the debugger the relativeURL argument to _getURL() called by the plugin is a string of total garbage. Isn't that call the one that's supposed to have the javascript: url in it? I'm filing this bug in case the fix for claim 2 (don't handle javascript: uris in Adobe) is unrelated to whatever is going on here.  Couldn't make this happen with long blah=http:// urls though.

Comment 1

[Daniel Veditz [:dveditz]]

Nominating for the upcoming releases because I think we need an answer here one way or another: either this is purely a problem in the plugin (invalid/wontfix) or we're messing up our own objects too (which we should fix).

If the former we just push Adobe 8 as the fix, if the latter it could be a potential problem with other plugins even after the Adobe fix.

Comment 2

[Johnny Stenback (:jst)]

The crash we're seeing here is indeed in the plugin code. But the reason for the crash is not necessarily strictly and only the plugins fault. What's happening here is that we initiate the plugin and at some point the plugin sees the blah=javascript:document.write('foo'); argument and asks us to load the URL. We then synchronously load that URL and do the requested document.write(), which in turn tears down the document containing the plugin and destroys the plugin, which is now in the the middle of requesting a URL load from the browser and it crashes during teardown in this case.

We could make either URL loading always be guaranteed to be async (which should fix this bug), or we could make plugin teardown be async (bug 347742). The latter is hard, and unlikely to happen for dot releases. The former is very scary since at least older versions of the flash plugin uses javascript: URL loading to fire off notifications to the page etc. Changing that for a dot release is not something I'd like to do unless there's no other way.

Comment 3

[Johnny Stenback (:jst)]

Attachment: Wrong diff.

This isn't tested yet, but it shows how we could do something like block certain protocols (or only permit some) for the acrobat plugin only. This would do the blocking for all versions of the acrobat plugin (as long as the plugin name contains both "Adobe" and "Acrobat"). We'd need to run this by the Adobe Acrobat plugin people etc before landing it, and once that's cleared up I can tweak this patch to do the exact blocking we want.

Comment 4

[Johnny Stenback (:jst)]

Attachment: Proof of concept for how this could be fixed for Adobe Acrobat only.

Comment 5

[Myk Melez [:myk] [@mykmelez]]

For what it's worth, Flash's getURL function doesn't appear to exhibit this problem.

Comment 6

[Johnny Stenback (:jst)]

Attachment: Proposed fix, per discussion with Adobe.

This should do what's necessary here, but I'm still waiting for final ok from Adobe.

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 252998 [details] [diff] [review]
Proposed fix, per discussion with Adobe.

I assume there must be name variations that prompt you to check for the words Adobe and Acrobat separately?

r/sr=dveditz

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 252998 [details] [diff] [review]
Proposed fix, per discussion with Adobe.

Go ahead and check these in on branches, Adobe will want to test a near-release candidate rather than a random trunk nightly.

a=dveditz for 1.8/1.8.0 branches

Comment 9

[Johnny Stenback (:jst)]

(In reply to comment #7)
> (From update of attachment 252998 [details] [diff] [review])
> I assume there must be name variations that prompt you to check for the words
> Adobe and Acrobat separately?

I'm not aware of one, but I don't have all old versions of Adobe Acrobat available and I'd rather play it safe at the minor performance hit of checking two separate strings.

Comment 10

[Johnny Stenback (:jst)]

Fixed on trunk and branches. FIXED.

Comment 11

[juan becerra [:juanb]]

Verified using information in first entry. First, I observed the warning and eventual hang on both 1509 and 2001 builds when entering the short/long urls. Then I tried on latest nightlies, 15010 and 2002, and I no longer see the problem. The pages load fine.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2pre) Gecko/20070206 BonEcho/2.0.0.2pre
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.10pre) Gecko/20070206 Firefox/1.5.0.10pre

Also verified it on latest trunk.