Possible Remote Code Execution from adobe reader (mem corruption)

RESOLVED FIXED

Status

()

Core
Plug-ins
--
critical
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: dveditz, Assigned: jst)

Tracking

({verified1.8.0.10, verified1.8.1.2})

Trunk
x86
Windows XP
verified1.8.0.10, verified1.8.1.2
Points:
---
Bug Flags:
blocking1.8.1.2 +
blocking1.8.0.10 +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:investigate] need adobe list = dveditz, URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

11 years ago
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.
(Reporter)

Comment 1

11 years ago
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.
Flags: blocking1.8.1.2?
Flags: blocking1.8.0.10?
Whiteboard: [sg:investigate]

Updated

11 years ago
Flags: blocking1.8.1.2?
Flags: blocking1.8.1.2+
Flags: blocking1.8.0.10?
Flags: blocking1.8.0.10+
(Assignee)

Comment 2

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

Comment 3

11 years ago
Created attachment 252119 [details] [diff] [review]
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.
(Assignee)

Updated

11 years ago
Attachment #252119 - Attachment description: Proof of concept for how this could be fixed for Adobe Acrobat only. → Wrong diff.
Attachment #252119 - Attachment is obsolete: true
(Assignee)

Comment 4

11 years ago
Created attachment 252120 [details] [diff] [review]
Proof of concept for how this could be fixed for Adobe Acrobat only.
(Assignee)

Updated

11 years ago
Attachment #252120 - Attachment is patch: true
Attachment #252120 - Attachment mime type: application/octet-stream → text/plain
For what it's worth, Flash's getURL function doesn't appear to exhibit this problem.
(Reporter)

Updated

11 years ago
Whiteboard: [sg:investigate] → [sg:investigate] need adobe list = dveditz
(Assignee)

Comment 6

11 years ago
Created attachment 252998 [details] [diff] [review]
Proposed fix, per discussion with Adobe.

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

Updated

11 years ago
Attachment #252998 - Flags: superreview?(dveditz)
Attachment #252998 - Flags: review?(dveditz)
(Reporter)

Comment 7

11 years ago
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
Attachment #252998 - Flags: superreview?(dveditz)
Attachment #252998 - Flags: superreview+
Attachment #252998 - Flags: review?(dveditz)
Attachment #252998 - Flags: review+
(Reporter)

Comment 8

11 years ago
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
Attachment #252998 - Flags: approval1.8.1.2+
Attachment #252998 - Flags: approval1.8.0.10+
(Assignee)

Comment 9

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

Comment 10

11 years ago
Fixed on trunk and branches. FIXED.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Keywords: fixed1.8.0.10, fixed1.8.1.2
Resolution: --- → FIXED
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.
Keywords: fixed1.8.0.10, fixed1.8.1.2 → verified1.8.0.10, verified1.8.1.2
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.