Closed Bug 297038 Opened 19 years ago Closed 19 years ago

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

Categories

(Firefox :: Security, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: matthew, Assigned: doronr)

References

Details

(Keywords: fixed-aviary1.0.5, Whiteboard: [sg:fix] need branch landing)

Attachments

(2 files)

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.
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?
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.
(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.
Attached 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
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
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?
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 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+
Whiteboard: [sg:fix] → [sg:fix] need branch landing
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
Closed: 19 years ago
Resolution: --- → FIXED
fix for bug 289940 checked into aviary and mozilla 1.7 branches
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
Flags: testcase+
Flags: in-testsuite+ → in-testsuite?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: