Closed Bug 288556 Opened 20 years ago Closed 19 years ago

Element embed pluginspage attribute allows javascript urls


(Core Graveyard :: Plug-ins, defect)

Windows ME
Not set


(Not tracked)



(Reporter: mromarkhan, Assigned: jst)



(Keywords: fixed-aviary1.0.3, Whiteboard: [sg:fix] fixed-trunk)


(5 files)

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.
Attached file Testcase
Saw bug bounty program rewarded today, so was a little bit too excited. Not
sure if dup or invalid bug.
Assignee: nobody → jst
Blocks: sbb?
Severity: normal → critical
Ever confirmed: true
Whiteboard: [sg:fix]
Attachment #179337 - Flags: superreview?(brendan)
Attachment #179337 - Flags: review?(darin)
Comment on attachment 179337 [details] [diff] [review]
Do security check when opening URL for manual plugin installation


maybe add a comment indicating why opener.content is being used.
Attachment #179337 - Flags: review?(darin) → review+
(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 on attachment 179341 [details] [diff] [review]
Same as above w/o the change which isn't necessary to fix this.

sr=me, noting darin's r=.

Attachment #179341 - Flags: superreview+
Attachment #179341 - Flags: review+
Johnny, can you provide your testcases here?
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.
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 loaded in
it. If you do, it still works.
Attachment #179360 - Attachment is patch: false
Attachment #179360 - Attachment mime type: text/plain → text/html
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 in new window.
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
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.
I have a patch for this issue as well - do I need to file a new bug or not?
filed bug 289171 with a patch on the 2nd issue.
Blocks: 289187
Fix released
Group: security
Flags: blocking1.8b2+
Flags: blocking-aviary1.1+
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.
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.
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?
I think this is fixed enough for 1.8b2, but leaving open for Johnny to do more
if he's still planning on it.
Whiteboard: [sg:fix] → [sg:fix] fixed-trunk
A cleaner way, without security manager checks would be:

nsPluginInstallerWizard.prototype.loadURL = function (aUrl){
  if (window.opener.getBrowser().mCurrentBrowser)

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.
Flags: blocking1.8b2+ → blocking1.8b3+
Please file a new bug on any remaining concerns. This bug as filed is fixed
Closed: 19 years ago
Resolution: --- → FIXED
Blocks: sbb+
No longer blocks: sbb?
Attachment #179337 - Flags: superreview?(brendan)
Flags: testcase+
Flags: in-testsuite+ → in-testsuite?
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.