Closed Bug 1527717 (CVE-2019-9801) Opened 8 months ago Closed 8 months ago

Windows programs that aren't "URL Handlers" exposed to web content

Categories

(Firefox :: File Handling, defect, P2)

Unspecified
Windows
defect

Tracking

()

RESOLVED FIXED
Firefox 67
Tracking Status
firefox-esr60 66+ fixed
firefox65 --- wontfix
firefox66 + fixed
firefox67 + fixed

People

(Reporter: dveditz, Assigned: rstrong)

References

Details

(Keywords: csectype-priv-escalation, regression, sec-moderate, Whiteboard: [post-critsmash-triage][adv-main66+][adv-esr60.6+])

Attachments

(1 file, 1 obsolete file)

On Windows Firefox will accept any registered ProgID as an external protocol scheme and offer to launch the local application if given such a URL. Unless a program has specifically advertised itself as a "URL Handler" (and presumably addressed the security implications of remote untrusted content) we should NOT offer to launch them.

For an example see https://crbug.com/889459 where a python.file: URL was used as a key part of a remote execution bug (with user interaction). IE/Edge, meanwhile, safely ignore python.file: (and a zillion others) because they're checking for the presence of the URL Handler key.

We'd want to do a similar check probably in
https://searchfox.org/mozilla-central/source/uriloader/exthandler/win/nsOSHelperAppService.cpp#72

Note: the Chrome RCE POC doesn't work in Firefox because, in part, we store downloads under a temp name until the user agrees to save it. But with a little more social engineering you could probably get around that. The point is that we don't know what dangerous program people will find next, and even if know there will be security bugs in programs that are URL Handlers (Firefox has them!), I'm even more sure there will be problems in programs that aren't expecting to be used that way.

Link to the Chrome bug was found on a Polish web security website so it may prompt researchers to play with variants.

Jim, Rob: who deals with the Windows external app handler these days?

Flags: needinfo?(robert.strong.bugs)
Flags: needinfo?(jmathies)
Flags: needinfo?(dveditz)

Maybe Jim knows but I have no idea.

Flags: needinfo?(robert.strong.bugs)

Rob's going to take a look, looks like a pretty easy fix.

Flags: needinfo?(jmathies)
Priority: -- → P2
Assignee: nobody → robert.strong.bugs
Status: NEW → ASSIGNED
Attached patch patch (obsolete) — Splinter Review

So much for this being simple... there are several tests that fail with this change
https://treeherder.mozilla.org/#/jobs?repo=try&revision=55e402aac8ff961d4a640b5b674f448afaa0028a

I think this could be due to the return for rv = regKey->Open(... being NS_FAILED when the key doesn't exist. Another try push
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0d40fe66876ba96ecb1faead28a41ebafdd459fd

That definitely fixes the tests I ran locally fingers crossed

Comment on attachment 9043853 [details]
Bug 1527717 - Check the registry for 'URL Protocol' when checking if a protocol handler exists. r?jmathies

Security Approval Request

How easily could an exploit be constructed based on the patch?

I don't think it could be done easily but it is difficult to know per comment #1

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

No

Which older supported branches are affected by this flaw?

60

If not all supported branches, which bug introduced the flaw?

Bug 1357791

Do you have backports for the affected branches?

No

If not, how different, hard to create, and risky will they be?

Extremely simple

How likely is this patch to cause regressions; how much testing does it need?

Not very likely though if clients learned to use this behavior removing it would be a surprise to them.

Attachment #9043853 - Flags: sec-approval?
Blocks: 1357791
Flags: needinfo?(dveditz)
Keywords: regression

Comment on attachment 9043853 [details]
Bug 1527717 - Check the registry for 'URL Protocol' when checking if a protocol handler exists. r?jmathies

sec-moderate and sec-low bugs don't need explicit approval on mozilla-central, but since I'm here looking sec-approval=dveditz

Is it worth uplifting to beta? sec-moderate and no known attacks say "no", but the recently fixed equivalent Chrome bug (which is somewhat worse combined with other issues) might trigger people poking around.

Attachment #9043853 - Flags: sec-approval? → sec-approval+

We'll want this on ESR-60 eventually. Thunderbird in particular might be at risk of malicious mails with links to launch unsafe applications.

I verified that the patch applies to both beta and esr60.

Group: firefox-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67

Comment on attachment 9043853 [details]
Bug 1527717 - Check the registry for 'URL Protocol' when checking if a protocol handler exists. r?jmathies

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

Bug 1357791

User impact if declined

Potential security exploit ( see comment #1 comment #10 and comment #11 ).

Is this code covered by automated tests?

Unknown

Has the fix been verified in Nightly?

Yes

Needs manual test from QE?

No

If yes, steps to reproduce

List of other uplifts needed

None

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Simple patch that restores functionality that existed prior to Firefox 60.

String changes made/needed

None

ESR Uplift Approval Request

If this is not a sec:{high,crit} bug, please state case for ESR consideration

Potential security exploit ( see comment #1 comment #10 and comment #11 ).

User impact if declined

Potential security exploit ( see comment #1 comment #10 and comment #11 ).

Fix Landed on Version

67

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Simple patch that restores functionality that existed prior to Firefox 60.

String or UUID changes made by this patch

None

Attachment #9043853 - Flags: approval-mozilla-esr60?
Attachment #9043853 - Flags: approval-mozilla-beta?

Comment on attachment 9043853 [details]
Bug 1527717 - Check the registry for 'URL Protocol' when checking if a protocol handler exists. r?jmathies

Blocks a potential vulnerability, OK to uplift for beta 8.

Attachment #9043853 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Adding checkin-needed for the beta landing

Keywords: checkin-needed

There is no need to set checkin-needed for uplifts to beta etc., the bugzilla queries for approval-mozilla-beta+ etc. catch those bugs.

https://hg.mozilla.org/releases/mozilla-beta/rev/bb9e3868a7bf0e1eeca0209d9943732ffb855958

Flags: qe-verify-
Whiteboard: [post-critsmash-triage]

Comment on attachment 9043853 [details]
Bug 1527717 - Check the registry for 'URL Protocol' when checking if a protocol handler exists. r?jmathies

Fixes a publicly-disclosed attack vector. Approved for 60.6esr.

Attachment #9043853 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main66+][adv-esr60.6+]
Alias: CVE-2019-9801
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.