288556 - Element embed pluginspage attribute allows javascript urls

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Omar Khan]

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]

Attachment: 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)]

Attachment: Do security check when opening URL for manual plugin installation

Comment 3

[Darin Fisher]

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)]

Attachment: Same as above w/o the content.open() change which isn't necessary to fix this.

Comment 5

[Johnny Stenback (:jst)]

(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]]

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]]

Johnny, can you provide your testcases here?

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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)]

Attachment: 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]]

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)]

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)]

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)]

Attachment: testcase for code injection

Comment 14

[Doron Rosenberg (IBM)]

I have a patch for this issue as well - do I need to file a new bug or not?

Comment 15

[Doron Rosenberg (IBM)]

filed bug 289171 with a patch on the 2nd issue.

Comment 16

[Daniel Veditz [:dveditz]]

Fix released

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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)]

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]]

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]]

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)]

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]]

Please file a new bug on any remaining concerns. This bug as filed is fixed
everywhere.