Insufficient validation in browser.js can lead to arbitrary code execution

RESOLVED FIXED

Status

()

defect
--
critical
RESOLVED FIXED
14 years ago
12 years ago

People

(Reporter: matthew, Assigned: doronr)

Tracking

({fixed-aviary1.0.5})

unspecified
Points:
---
Dependency tree / graph
Bug Flags:
blocking-aviary1.0.5 +
blocking1.8b3 +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:fix] need branch landing)

Attachments

(2 attachments)

Reporter

Description

14 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.8) Gecko/20050513 Fedora/1.0.4-1.3.1 Firefox/1.0.4
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.8) Gecko/20050513 Fedora/1.0.4-1.3.1 Firefox/1.0.4

The browser exposes a "PluginNotFound" event to content, but doesn't validate
that the event is trusted.  It then proceeds to link a click event to the
element that "PluginNotFound" was fired on and then uses that element to
populate the target of a button.

By overriding toString() on an object, we can then avoid the javascript: url
check in the plugininstallerwindow at:

http://lxr.mozilla.org/aviary101branch/source/toolkit/mozapps/plugins/content/pluginInstallerWizard.js#512

In all, these exploits allow us to run arbitrary Javascript with chrome
privileges when the user clicks the "manual install" button.

See the attached testcase.

Reproducible: Always

Steps to Reproduce:
Load testcase.  
Actual Results:  
Chrome-privileged object displayed.
Reporter

Comment 1

14 years ago
This is the sample testcase that shows the problem.  The exploit happens during
the onload phase.
So on trunk this is blocked by the general trusted event changes, right?  So
it's a matter of adding an isTrusted check on branch?
Assignee

Comment 3

14 years ago
We really need to find a better way than adding an event handler, but even then
we would still need to do a security check probably.

So basically, all I need to do is add the following, right?

if (!event.isTrusted) {
  // bail out
}

I'll post/test this when I get into work, don't have a branch tree at home.
Reporter

Comment 4

14 years ago
(In reply to comment #3)

> So basically, all I need to do is add the following, right?
> 
> if (!event.isTrusted) {
>   // bail out
> }

One additional note to my bug report: the events in both
missingPluginInstaller.prototype.newMissingPlugin and
missingPluginInstaller.prototype.installSinglePlugin will need to checked for
trustworthiness to completely mitigate this bug.

This bug is technically two - one lets us synthesize a missing plugin and the
other lets us similate a click on the other to start the plugin search process.
Assignee

Comment 5

14 years ago
Posted patch le patchSplinter Review
My avairy tree isn't up to date, but this patch fixes the testcase.

Updating my tree right now.
Assignee: nobody → doronr
Status: UNCONFIRMED → ASSIGNED
Attachment #185670 - Flags: review?(mconnor)
Comment on attachment 185670 [details] [diff] [review]
le patch

s/trust-worthy/trustworthy/
Attachment #185670 - Flags: review?(mconnor) → review+
The trunk is protected by jst's trusted event patch (bug 289940).  We should
land that patch on the branch, and dveditz is doing exactly that.  Noting
version and marking dependency.

/be
Depends on: 289940
Version: unspecified → 1.0 Branch
Assignee

Comment 8

14 years ago
So this patch should be invalidated then, right?
Fix one way or the other. The patch for bug 289940 has been tough to merge,
might be riskier than we want for the branch.
Flags: blocking1.8b3+
Flags: blocking-aviary1.0.5+
Whiteboard: [sg:fix]
Version: 1.0 Branch → unspecified
Assignee

Comment 10

14 years ago
Comment on attachment 185670 [details] [diff] [review]
le patch

Asking for 1.0.5 approval, depending on how we want to fix this.
Attachment #185670 - Flags: approval-aviary1.0.5?
Assignee

Comment 11

14 years ago
dveditz - any progress with the event patch?  This bug could be closed if that
is going to go in and reduce the 1.0.5 list :)

Comment 12

14 years ago
Comment on attachment 185670 [details] [diff] [review]
le patch

dveditz patch is going to get checked in soon, but per drivers meeting, let's
get this patch in as well. a=jay
Attachment #185670 - Flags: approval-aviary1.0.5? → approval-aviary1.0.5+

Updated

14 years ago
Whiteboard: [sg:fix] → [sg:fix] need branch landing
Assignee

Comment 13

14 years ago
I'll check it in tomorrow, don't have a aviary tree at home.

Does this need to go into trunk?  If yes, do I need another a=?
checked in for doron
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
fix for bug 289940 checked into aviary and mozilla 1.7 branches

Comment 16

14 years ago
v.fixed on aviary with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.9)
Gecko/20050706 Firefox/1.0.5 using original testcase.
Adding distributors
FF1.0.5 advisories published
Group: security

Updated

14 years ago
Flags: testcase+

Updated

12 years ago
Flags: in-testsuite+ → in-testsuite?
You need to log in before you can comment on or make changes to this bug.