Last Comment Bug 288556 - Element embed pluginspage attribute allows javascript urls
: Element embed pluginspage attribute allows javascript urls
Status: RESOLVED FIXED
[sg:fix] fixed-trunk
: fixed-aviary1.0.3
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: x86 Windows ME
: -- critical (vote)
: ---
Assigned To: Johnny Stenback (:jst, jst@mozilla.com)
:
Mentors:
Depends on:
Blocks: sbb+ 289187
  Show dependency treegraph
 
Reported: 2005-03-31 19:10 PST by Omar Khan
Modified: 2007-04-01 14:35 PDT (History)
16 users (show)
asa: blocking1.8b3+
dveditz: blocking‑aviary1.5+
bob: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Testcase (610 bytes, text/html)
2005-03-31 19:31 PST, Omar Khan
no flags Details
Do security check when opening URL for manual plugin installation (3.22 KB, patch)
2005-04-01 17:30 PST, Johnny Stenback (:jst, jst@mozilla.com)
darin.moz: review+
Details | Diff | Review
Same as above w/o the content.open() change which isn't necessary to fix this. (3.18 KB, patch)
2005-04-01 17:57 PST, Johnny Stenback (:jst, jst@mozilla.com)
brendan: review+
brendan: superreview+
dbaron: approval‑aviary1.0.3+
Details | Diff | Review
Testcase showing manual installation still works. (364 bytes, text/html)
2005-04-01 21:57 PST, Johnny Stenback (:jst, jst@mozilla.com)
no flags Details
testcase for code injection (241 bytes, text/html)
2005-04-05 09:58 PDT, Doron Rosenberg (IBM)
no flags Details

Description Omar Khan 2005-03-31 19:10:25 PST
User-Agent:       Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.8b2) Gecko/20050331 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.8b2) Gecko/20050331 Firefox/1.0+

Element embed pluginspage attribute allows javascript urls, and somehow
somebody forgot to santize it.
So can execute arbitrary code.

Reproducible: Always

Steps to Reproduce:
1. Load page with an embed's pluginspage attribute set to javascript url
2. Click install missing plugins
3. Click Manual install

Actual Results:  
Arbitrary code executed

Expected Results:  
Do not execute arbitrary code.
Thank you.
Comment 1 Omar Khan 2005-03-31 19:31:23 PST
Created attachment 179237 [details]
Testcase

Saw bug bounty program rewarded today, so was a little bit too excited. Not
sure if dup or invalid bug.
Comment 2 Johnny Stenback (:jst, jst@mozilla.com) 2005-04-01 17:30:03 PST
Created attachment 179337 [details] [diff] [review]
Do security check when opening URL for manual plugin installation
Comment 3 Darin Fisher 2005-04-01 17:45:43 PST
Comment on attachment 179337 [details] [diff] [review]
Do security check when opening URL for manual plugin installation

r=darin

maybe add a comment indicating why opener.content is being used.
Comment 4 Johnny Stenback (:jst, jst@mozilla.com) 2005-04-01 17:57:08 PST
Created attachment 179341 [details] [diff] [review]
Same as above w/o the content.open() change which isn't necessary to fix this.
Comment 5 Johnny Stenback (:jst, jst@mozilla.com) 2005-04-01 17:58:11 PST
(In reply to comment #3)
> (From update of attachment 179337 [details] [diff] [review] [edit])
> r=darin
> 
> maybe add a comment indicating why opener.content is being used.
> 

Yeah, but on second thought I desided to not risk changing that now. I'll worry
about that later, so ignore that part of the first patch.
Comment 6 Brendan Eich [:brendan] 2005-04-01 18:36:11 PST
Comment on attachment 179341 [details] [diff] [review]
Same as above w/o the content.open() change which isn't necessary to fix this.

sr=me, noting darin's r=.

/be
Comment 7 Asa Dotzler [:asa] 2005-04-01 19:00:50 PST
Johnny, can you provide your testcases here?
Comment 8 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2005-04-01 20:55:30 PST
Checked in to AVIARY_1_0_20050124_BRANCH, 2005-04-01 18:48 -0800.

Note:  this was *not* yet checked in to the trunk.  I'll let jst do that,
perhaps once we're ready to tolerate a little more exposure.
Comment 9 Johnny Stenback (:jst, jst@mozilla.com) 2005-04-01 21:57:16 PST
Created attachment 179360 [details]
Testcase showing manual installation still works.

This is a testcase (based on the original testcase in this bug) that shows that
manual installation still works with non-evil pluginspage attribute values.
Load the testcase, click the missing plugin and click on the manual
installation URL and you should get a new window with www.mozilla.org loaded in
it. If you do, it still works.
Comment 10 Jay Patel [:jay] 2005-04-02 00:51:47 PST
Verified fixed on Aviary branch - Mozilla/5.0 (Windows; U; Windows NT 5.1;
en-US; rv:1.7.7) Gecko/20050401 Firefox/1.0.3

Original testcase:  Manual plugin install no longer changes my homepage.
jst's testcase: Works as expected, opens mozilla.org in new window.
Comment 11 sairuh (rarely reading bugmail) 2005-04-04 11:10:16 PDT
verified fixed 200504030x-1.0.3 on linux fc3 and mac os x. same results as in
comment 10: the first testcase doesn't reset my homepage, and jst's testcase
opens www.mozilla.org.
Comment 12 Doron Rosenberg (IBM) 2005-04-05 09:56:38 PDT
I just figured out another way to exploit this that isn't fix by the patch.  It
can be used to inject code into the xul:button's oncommand.
Comment 13 Doron Rosenberg (IBM) 2005-04-05 09:58:26 PDT
Created attachment 179724 [details]
testcase for code injection
Comment 14 Doron Rosenberg (IBM) 2005-04-05 11:42:58 PDT
I have a patch for this issue as well - do I need to file a new bug or not?
Comment 15 Doron Rosenberg (IBM) 2005-04-05 12:07:50 PDT
filed bug 289171 with a patch on the 2nd issue.
Comment 16 Daniel Veditz [:dveditz] 2005-04-15 19:50:41 PDT
Fix released
Comment 17 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2005-04-15 21:47:07 PDT
Checked in to trunk, but leaving open since I suspect jst wanted to get the
difference between the two patches in on the trunk too.
Comment 18 Doron Rosenberg (IBM) 2005-04-16 06:26:54 PDT
I think for trunk we can do better and just use the window that contains the
missing plugin (the regular html dom window, not chrome window) to open the new
window, and thus skip the nsIScriptSecurityManager check.

I'll look into it next week.
Comment 19 Asa Dotzler [:asa] 2005-04-17 11:32:13 PDT
Are we done with the "blocking1.8b2+" part of this fix? If so, can we move
remaining work to blocking 1.8b3+ and unset that flag or do we have more to do
here for b2?
Comment 20 Mike Connor [:mconnor] 2005-04-21 14:42:08 PDT
I think this is fixed enough for 1.8b2, but leaving open for Johnny to do more
if he's still planning on it.
Comment 21 Doron Rosenberg (IBM) 2005-04-21 15:24:57 PDT
A cleaner way, without security manager checks would be:

nsPluginInstallerWizard.prototype.loadURL = function (aUrl){
  if (window.opener.getBrowser().mCurrentBrowser)
    window.opener.getBrowser().mCurrentBrowser.contentWindow.open(aUrl);
}

use the current browser's contentWinow to open the url.  This should be safe,
thought perhaps if the current browser is pointing at an chrome:// url this
could cause trouble.  Probably needs more testing.
Comment 22 Daniel Veditz [:dveditz] 2005-05-20 17:26:20 PDT
Please file a new bug on any remaining concerns. This bug as filed is fixed
everywhere.

Note You need to log in before you can comment on or make changes to this bug.