1552627 - (CVE-2019-11702) Lack of mitigation on external protocol execution (res: protocol)

Status: VERIFIED FIXED

(Core :: Networking, defect, P2)

Description

[Daniel Veditz [:dveditz]]

+++ This bug was initially created as a clone of Bug #1549833 +++

Essentially the same as bug 1549833, except you can launch IE with the IE.HTTP: scheme. Example:

IE.HTTP:res://apds.dll/redirect.html?target=javascript:alert(1)

IE.HTTP: sends the string to IE, and then all the res: payloads from the other bugs work again.

I don't know if there are more IE aliases, but there are a lot of URL Protocols that look at least somewhat risky. Maybe while we're here we can change the default button the external handler dialog to "Cancel" to at least make it a little harder to do by accident (e.g. holding down enter). Could also double the button disable delay -- it's really fast.<

Comment 1

[Daniel Veditz [:dveditz]]

from bug 1549833 comment 3

Are there other protocols you're aware of that need restricting?

Sorry, Gijs, here's one.

Pascal: if we just pref-flip IE.HTTP like the patch in bug 1549833 is there any chance to get this into Firefox 67 or a ridealong 67.0.x? Once we release an advisory for the other bug people will start looking and this isn't that hard to find.

Comment 2

[:Gijs (he/him)]

(In reply to Daniel Veditz [:dveditz] from comment #0)

Could also double the button disable delay

It should be the same as other dialogs, from pref security.dialog_enable_delay. Is it not, in practice?

Comment 3

[:Gijs (he/him)]

Also, how deep does this rabbithole go - in my win10 machine, there's also IE.FTP and IE.HTTPS, but they're not marked as URL protocols. There's also iehistory and ierss which are marked as URL protocols but don't seem to "work" as they don't have an associated app (at least on the 1 system where I checked this).

Comment 4

[Pascal Chevrel:pascalc]

(In reply to Daniel Veditz [:dveditz] from comment #1)

from bug 1549833 comment 3

Are there other protocols you're aware of that need restricting?

Sorry, Gijs, here's one.

Pascal: if we just pref-flip IE.HTTP like the patch in bug 1549833 is there any chance to get this into Firefox 67 or a ridealong 67.0.x? Once we release an advisory for the other bug people will start looking and this isn't that hard to find.

As a ridealong for a 67 dot release (not the trailhead one), yes, but for 67.0 it's too late.

Comment 5

[:Gijs (he/him)]

Attachment: Bug 1552627 - r?dveditz

Comment 6

[:Gijs (he/him)]

Attachment: Bug 1552627 - pt 2, r?dveditz

Depends on D31722

Comment 7

[:Gijs (he/him)]

So the delay is 1s, decided on in bug 916726 . It was 2s before, but that was deemed too long, so I'm not really inclined to increase the timeout for all dialogs. We could separate the timeout for this dialog from the default, if that was deemed necessary.

I'm attaching 1 patch that:

• blocks ie.http (but not ie.https/ie.ftp which don't seem to be protocols on my machine?)
• blocks iehistory/ierss (despite not having an association on my machine - they just look dodgy)

and another patch that prolongs the 1s timeout from repeated keydown events (this affects all users of the EnableDelayHelper class), and moves the acceptance of this specific dialog away from an "accept" button (so enter doesn't trigger it in the first place).

There's still a potential for accepting the dialog if you encourage users to mouseclick somewhere, though that'd require being able to predict the positioning of the dialog and the buttons which seems much more error prone, and keeping them clicking for >=1s after the dialog has appeared. I don't know if that's enough to justify changing the delays here. If so please let me know.

Comment 8

[Daniel Veditz [:dveditz]]

(In reply to :Gijs (he/him) from comment #2)

It should be the same as other dialogs, from pref security.dialog_enable_delay. Is it not, in practice?

It is the same. Felt fast but maybe I was just having a slow day.

Agreed that iehistory/ierss looked dodgy, but I couldn't make them do anything so left them off. At first glance you'd assume IE.HTTPS and IE.FTP work like IE.HTTP, but we no longer recognize them from Firefox thanks to bug 1527717.

There's still a potential for accepting the dialog if you encourage users to mouseclick somewhere [....] I don't know if that's enough to justify changing the delays here.

The delay is more than enough for it's original purpose of preventing surprise clicks by popping the prompt up under the spot where you've trained/lured a user to click. They're looking at the spot and will see the dialog appear and can react. The "hold a key down" attack is harder to prevent because the attacker can get a user to focus somewhere away from where the dialog pops up so they might not let up in time. Time delay isn't the fix for that. For that we should change the default focus to Cancel, and make sure we don't have a Hot Key for the dangerous button.

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/autoland/rev/29d9eb1276a71d4f4f1edefe4b0e720e4b16f952
https://hg.mozilla.org/integration/autoland/rev/6fefc560ddc453a3a41dc1048a917f0cc0d69655
https://hg.mozilla.org/mozilla-central/rev/29d9eb1276a7
https://hg.mozilla.org/mozilla-central/rev/6fefc560ddc4

Comment 10

[:Gijs (he/him)]

I'm waiting at least a little bit before requesting uplift to 68 because I'm a little worried about regressions - there don't seem to be any automated tests covering this code... Though if we want this for a ride-along I should probably take timing of 67.0.5 into account. Worst case we could only block the extra protocols and have the changes to the dialog ride with 68.

Pascal, what's the timeline for 67.0.5 / dot releases?

Comment 11

[Pascal Chevrel:pascalc]

(In reply to :Gijs (he/him) from comment #10)

Pascal, what's the timeline for 67.0.5 / dot releases?

67.0.5 (Trailhead) ships on June 4. Note that 67.0.5 is a placeholder version for the Trailhead release, it's likely to be called 67.0.1 or 67.0.2 if we have a dot release before. We don't take ride alongs in the Trailhead release, we know that we will have a non-trailhead dot release but at the moment, one day after shipping 67, we don't have a strong driver for a dot release in the days to come.

Comment 12

[:Gijs (he/him)]

Comment on attachment 9065879 [details]
Bug 1552627 - r?dveditz

Beta/Release Uplift Approval Request

• User impact if declined: Security risks
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: I will attach 2 testcases with instructions.
• List of other uplifts needed: n/a
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): For beta, I think this is low-risk. We have a lot of beta runway, the changes are relatively straightforward and all frontend-only, and beta will help flush out regressions -- the protocol dialog doesn't really come up every day and the nightly population is much smaller. The worst thing that could happen is inadvertent breakage of time-delay'd dialogs/notifications.
• String changes made/needed: nope

Comment 13

[:Gijs (he/him)]

Attachment: Testcase 1

This is the simplest testcase.

Pre-patch: opening this testcase shows a dialog on Windows that offers to open Internet Explorer. It will likely do nothing or show an error page on mac/linux.

Expected post-patch: opening the testcase shows a blank or error page (no more dialog to open IE.

Comment 14

[:Gijs (he/him)]

Attachment: Testcase 1

Bugzilla is not playing ball - hopefully this testcase works? You'd need to click the button for anything to happen.

Comment 15

[:Gijs (he/him)]

Attachment: Testcase 2

OK, second testcase...

Open this, use the keyboard to focus the button (ie use Tab key; on mac you may need to turn on "full keyboard access" in the system preferences), then hold down enter

Pre-patch: the dialog to open the telnet link will come up, and after some 1s delay, the selected application will open (for me, this is Terminal on Mac, and "Internet shortcut shell extension dll" on Windows 10 - I've not tested Linux, it's possible there's no protocol handler there, in which case, ignore Linux)

Post-patch, expected: the dialog will open, but the selected application will not open by just holding down enter - the "Open Link" button will remain disabled until shortly after you stop holding down enter.

Comment 16

[Rares Doghi, Desktop QA]

Using the test case 2 and steps from Comment 15 I can confirm this issue Verified as Fixed on Mac and Windows 10 using the latest version of Nightly 69.0a1 (2019-05-24). I was able to reproduce it on release and Beta.

Comment 17

[Julien Cristau [:jcristau]]

Comment on attachment 9065879 [details]
Bug 1552627 - r?dveditz

sec fix, verified in nightly, approved for 68.0b5

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/dfc5b049c351cf99e7bddd88499d23c48659283b
https://hg.mozilla.org/releases/mozilla-beta/rev/27c8512c0e991930fa7bb01c6a89561772a64e48

Comment 19

[:Gijs (he/him)]

Comment on attachment 9065879 [details]
Bug 1552627 - r?dveditz

Beta/Release Uplift Approval Request

• User impact if declined: Security risks
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See earlier comments.
• List of other uplifts needed: n/a
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): For release, I think this is medium-to-low-risk, considering we're talking about release. The changes are contained, frontend only, and have been on nightly and beta a while now. If we're nervous about this, we could take only the pt2 patch (which is approx. 0 risk as it just flips some prefs).
• String changes made/needed: Nope

Comment 20

[Pascal Chevrel:pascalc]

Comment on attachment 9065879 [details]
Bug 1552627 - r?dveditz

approved for 67.0.2, thanks.

Comment 21

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-release/rev/a33f8698b1b9
https://hg.mozilla.org/releases/mozilla-release/rev/0b7cdeda8ce9

Comment 22

[Rares Doghi, Desktop QA]

This issue is verified as fixed in Beta 68.0b9 (64-bit) as well as Release 67.0.2. I will mark this issue accordingly.

Comment 23

[Daniel Veditz [:dveditz]]

Nominating for a security bug bounty at the reporter's request

Comment 24

[Al Billings [:abillings - ex-MoCo]]

This does not qualify for a bounty at this point because it was derived from an earlier bug, for which a bounty was paid, as part of due diligence by Mozilla staff looking at the full of scope of vulnerability.