1447080 - (CVE-2018-5174) Security: SEE_MASK_FLAG_NO_UI behavior changes in Windows 10, allowing SmartScreen bypass

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect, P1)

Description

[Jimmy]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.162 Safari/537.36

Steps to reproduce:

Download and run these files:
Known malware: https://demo.smartscreen.msft.net/download/known/knownmalicious.exe
Unknown program: https://demo.smartscreen.msft.net/download/unknown/freevideo.exe
Offline: Any of the above with internet on the machine off

More details about the issue:
In the next Windows Release (Redstone 4), Windows Defender SmartScreen will honor the SEE_MASK_FLAG_NO_UI flag. This flag is passed through shellexecute when a user runs an executable, or other supported file, through the firefox download manager. Edge/Windows Explorer do not pass this flag and are unaffected.

There are three problematic scenarios
1. Known malware - Blocked silently: We will not show any SmartScreen UI for these types of files, but the user will have no opportunity to override the block through firefox. The user will not have any indication that executing the file did anything.
2. Unknowns - The file is unknown to our service. This is a large number of files and all will be allowed. There is a chance for polymorphic malware to be bucketed here.
3. Offline - User is unable to contact smartscreen. Always allowed.

When SEE_MASK_FLAG_NO_UI is not set, all 3 cases will result in a block experience for the user.


Actual results:

Files are not run. They are silently blocked


Expected results:

Files are not run. They are blocked by SmartScreen.

Comment 1

[:Gijs (out for now; he/him)]

As best I can tell this was added in bug 394974 for Firefox 2.0 (hi, 2006 called, it wants its flags back, etc.).

Jim, can we just drop this flag? Is there an alternative we should be using? It's not clear from bug 394974 why this flag was added (though some of the others are more obvious).

I expect we'll want to have a fix in place for 60 given the release timelines for the spring updates in the past (april last year for that one). I don't know how we feel about 59 for this.

Comment 2

[Jim Mathies [:jimm]]

(In reply to :Gijs from comment #1)
> As best I can tell this was added in bug 394974 for Firefox 2.0 (hi, 2006
> called, it wants its flags back, etc.).
> 
> Jim, can we just drop this flag? Is there an alternative we should be using?
> It's not clear from bug 394974 why this flag was added (though some of the
> others are more obvious).
> 
> I expect we'll want to have a fix in place for 60 given the release
> timelines for the spring updates in the past (april last year for that one).
> I don't know how we feel about 59 for this.

I think I just added that because it seemed like a good flag to pass. I don't remember any other specifics.

Comment 3

[Jim Mathies [:jimm]]

Who is [e-mail] ? This report sounds like it was filed by a Microsoft employee, but I don't recognize the account.

Comment 4

[Daniel Veditz [:dveditz]]

I have no idea, but removing the flag and showing errors seems like a good thing regardless.

I don't know why this is a hidden security bug or described as a "bypass". From the description it sounds like things that SmartScreen would block are blocked, just silently, and additional things SmartScreen wouldn't block (offline) are also blocked -- perhaps a bug but not going to cause any security issues.

Comment 5

[Jimmy]

Hey, I'm a dev on the smartscreen team. Sorry, it looks like I didn't update the "actual results" properly.

The security issue is the bucket of unknowns and offline.

The unknown bucket has the potential to contain things like polymorphic malware and will be allowed

Offline: running files offline will also be allowed. These could be files that would normally be blocked by smartscreen.

Comment 6

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

Dan, should we open this?

Comment 7

[Daniel Veditz [:dveditz]]

jafeher: I wrote a "User Story" to replace your original description that hopefully removes the confusion. Did I get it right?

(meanwhile I'll set a security rating assuming it is)

Comment 8

[Daniel Veditz [:dveditz]]

Jim: we'd like this fixed in 60 so it's not too behind the Windows release. Who owns this code now?

Comment 9

[Jim Mathies [:jimm]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9fe49528449fc627b96fe03ebe4757bbf67fcac1

Comment 10

[Jim Mathies [:jimm]]

Attachment: patch v.1

Comment 11

[Jim Mathies [:jimm]]

Comment on attachment 8962342 [details] [diff] [review]
patch v.1

David mind looking this one line change over? I did a little research on this flag, didn't find anything out on the net indicating this change might introduce some sort of issue. Generally looks like a safe change although I do wonder if users will start seeing error messages when executing attachments due to this. If so, hopefully those messages are useful. Not sure we want to push this out fast, I think lots of bake time is warranted here.

Comment 12

[Jim Mathies [:jimm]]

Comment on attachment 8962342 [details] [diff] [review]
patch v.1

Putting this on hold after discussing with David. We're not sure we want to make these changes. Blocking #2 in particular sounds like it would produce really awful user experiences. #3 is questionable as well.

Comment 13

[Jimmy]

User story looks correct.

@Jim This is how SmartScreen has functioned previously. Chrome has agreed to making this same change. Any commonly downloaded file quickly get reputation in smartscreen and will be allowed. Clicking through SmartScreen once will clear mark of the web and that file will not be blocked again.

Let me know if you require any more information.

Comment 14

[Jim Mathies [:jimm]]

SmartScreen description off wikipedia - 

"It is designed to help protect users against attacks that utilize social engineering and drive-by downloads to infect a system by scanning URLs accessed by a user against a blacklist of websites containing known threats."

https://en.wikipedia.org/wiki/Microsoft_SmartScreen

Comment 15

[Jim Mathies [:jimm]]

Hey Romain, can Product provide some input here? Sounds like we probably want to ship this flag but there's an open question as to how annoying it might be for the user. We could tie this to a hidden pref if we think users might want a way to disable. Although I'd prefer not adding that unless we absolutely need it.

Comment 16

[Jim Mathies [:jimm]]

Attachment: Win7 SmartScreen warning

Comment 17

[Romain Testard [:RT]]

(In reply to Jim Mathies [:jimm] from comment #15)
> Hey Romain, can Product provide some input here? Sounds like we probably
> want to ship this flag but there's an open question as to how annoying it
> might be for the user. We could tie this to a hidden pref if we think users
> might want a way to disable. Although I'd prefer not adding that unless we
> absolutely need it.

My understanding is that this will only impact Win10 users updating to Redstone 4 (Released Apr 10th it seems).
I tested the files provided in the user story (I run Redstone 3 with Firefox 59.0.2) and both get me a warning screen (red for the known malware and blue for the unknown program) when I attempt to run them (see attached). Can you clarify what is the actual Win10 user experience if SEE_MASK_FLAG_NO_UI is *not* set with Redstone 4 ?

Also do we have telemetry for how often we set that flag on Win10?

Comment 18

[Romain Testard [:RT]]

Attachment: knownmalicious.exe on win10

knownmalicious.exe on win10

Comment 19

[Romain Testard [:RT]]

Attachment: freevideo.exe on win10

Comment 20

[David Parks [:handyman]]

FWIW, I'd add that the "unknown program" case Romain speaks of (the freevideo.exe one) requires the user to click "More Information" to get to a prompt that allows them to run the program anyway.  Since Firefox is my browser, I very rarely see this prompt so it trips me up every time.  It takes me a moment to figure out what to do and I already know what to do.

Comment 21

[Jim Mathies [:jimm]]

(In reply to Romain Testard [:RT] from comment #17)
> (In reply to Jim Mathies [:jimm] from comment #15)
> > Hey Romain, can Product provide some input here? Sounds like we probably
> > want to ship this flag but there's an open question as to how annoying it
> > might be for the user. We could tie this to a hidden pref if we think users
> > might want a way to disable. Although I'd prefer not adding that unless we
> > absolutely need it.
> 
> My understanding is that this will only impact Win10 users updating to
> Redstone 4 (Released Apr 10th it seems).

Correct.

> I tested the files provided in the user story (I run Redstone 3 with Firefox
> 59.0.2) and both get me a warning screen (red for the known malware and blue
> for the unknown program) when I attempt to run them (see attached). Can you
> clarify what is the actual Win10 user experience if SEE_MASK_FLAG_NO_UI is
> *not* set with Redstone 4 ?

According to Microsoft the second two test cases would result in execution without a prompt starting in redstone 4. We still get a block in #1, but apparently we won't get the Windows warning dialog, it'll just fail silently.

> Also do we have telemetry for how often we set that flag on Win10?

We pass this to the API call that launches 3rd party apps for us, so we always pass it. The question is do we want to stop doing that.

The more I dig into this, the more I'm convinced we should take this change.

Comment 22

[Romain Testard [:RT]]

Thanks. Talking about experience change between current and Redstone 4 with flag removed my understanding is it will be:
1 Known malware: currently the user sees the red screen, in Redstone 4 with flag removed the user would just see nothing (silent fail. This sounds fine to me if the security risk warrants preventing the user from running the file
2 Unknown program: currently the user sees the blue screen, in Redstone 4 with flag removed the user would still see the blue screen (no change). (I understand from the user story that there is a block in this case and I assume the blue screen as UI)

It sounds to me like we should implement the change since the user experience is only impacted in (1) in a case where known malware would now be prevented from running.

My question above on telemetry was to understand how often the average user sees the screen in scenario (2) although if my interpretation is right this will not change assuming we remove the flag so no need to look into this.

Comment 23

[Jim Mathies [:jimm]]

(In reply to Romain Testard [:RT] from comment #22)
> Thanks. Talking about experience change between current and Redstone 4 with
> flag removed my understanding is it will be:
> 1 Known malware: currently the user sees the red screen, in Redstone 4 with
> flag removed the user would just see nothing (silent fail. This sounds fine
> to me if the security risk warrants preventing the user from running the file

This is incorrect. Currently Firefox will trigger a blocked execution prompt in pre-redstone4 versions of Windows. In redstone4, (according to our ms friend here) current Firefox will fail silently. Landing the patch here (removing the "don't show ui" flag) will not impact Firefox behavior on pre-redtone4, with redstone4 landing the patch will trigger a blocked execution prompt.

> 2 Unknown program: currently the user sees the blue screen, in Redstone 4
> with flag removed the user would still see the blue screen (no change). (I
> understand from the user story that there is a block in this case and I
> assume the blue screen as UI)

Reversed again, but generally yes we want to fix the issue where the blue screen might not display. Landing this patch will accomplish this.

> It sounds to me like we should implement the change since the user
> experience is only impacted in (1) in a case where known malware would now
> be prevented from running.

Ok, we're going land this such that on redstone 4 and beyond, our users get prompted with warnings in all three cases here. Without this patch, those warnings would not be displayed (as they currently are in pre-redstone4 releases of Windows.)

Comment 24

[Jim Mathies [:jimm]]

Comment on attachment 8962342 [details] [diff] [review]
patch v.1

We've decided to take this.

Comment 25

[David Parks [:handyman]]

Comment on attachment 8962342 [details] [diff] [review]
patch v.1

Review of attachment 8962342 [details] [diff] [review]:
-----------------------------------------------------------------

Agreed.  Its too valuable to go without.

Comment 26

[Jim Mathies [:jimm]]

Comment on attachment 8962342 [details] [diff] [review]
patch v.1

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
This is not a vulnerability bug. Flag change that impacts operating system security response to 3rd party content handling via Firefox. 
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?
all
If not all supported branches, which bug introduced the flaw?
bug 394974
Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
no, and this is not needed. we're going to fix this in current release, and uplift to 60 ESR.
How likely is this patch to cause regressions; how much testing does it need?
medium?

Comment 27

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

Comment on attachment 8962342 [details] [diff] [review]
patch v.1

As a sec-moderate, this doesn't need sec-approval to land. Let's get it onto trunk and then see about uplifting it to Beta. That will need to happen within the next two weeks so we have a couple of beta left before we ship 60.

Comment 28

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/56bfd2178dcad46a5b611b14e31938f1d96124a0

Comment 29

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

https://hg.mozilla.org/mozilla-central/rev/56bfd2178dca

Comment 30

[Ryan VanderMeulen [:RyanVM]]

Seems worth a Beta uplift request at least. What are your thoughts on ESR52 (EOL in late August)?

Comment 31

[Jim Mathies [:jimm]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #30)
> Seems worth a Beta uplift request at least. What are your thoughts on ESR52
> (EOL in late August)?

Once we've verified this is 'ok', this patch can ship to pretty much any release we have right now.

Comment 32

[Jim Mathies [:jimm]]

Comment on attachment 8962342 [details] [diff] [review]
patch v.1

Approval Request Comment
[Feature/Bug causing the regression]:
A newer version of Windows will ship feature changes that are impacted by this flag.
[User impact if declined]:
No warning UI when users interact with untrusted files. 
[Is this code covered by automated tests?]:
yes.
[Has the fix been verified in Nightly?]:
No, we probably should ask for pi on the affected windows version. I will file a pi request.
[Needs manual test from QE? If yes, steps to reproduce]:
see user story 
[List of other uplifts needed for the feature/fix]:
none
[Is the change risky?]:
no
[Why is the change risky/not risky?]:
well understood side effects.
[String changes made/needed]:
none.

Comment 33

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8962342 [details] [diff] [review]
patch v.1

Approved for 60.0b13. Per Jim's PI request, let's get some extra QA attention on it as well.

Comment 34

[Ryan VanderMeulen [:RyanVM]]

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

Comment 35

[Jim Mathies [:jimm]]

Hey jafeher, We're not seeing the prompting in Windows version 1803 (OS Build 17133.73) with our updated version of Firefox. Just want to confirm that the new behavior you describe is in the Windows build above.

Comment 36

[Jimmy]

Yes this behavior should exist in that build.

Is this change available in any public beta version of firefox that I can test?

Comment 37

[Ryan VanderMeulen [:RyanVM]]

Yes, 60.0b13 was released today with this fix included.
https://www.mozilla.org/en-US/firefox/channel/desktop/

Comment 38

[Jimmy]

Thanks for the information.

We did not realize that Firefox/Chrome call ShellexecuteExW in different ways. There is also a Windows bug in the path that firefox calls. We are working on a fix and will get this out in an early servicing patch.

Let me know if you require any other information.

Comment 39

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

This seems to have missed the ESR52 train as well despite being tracking for 60+ and affected.

Comment 40

[Liz Henry (:lizzard) (relman/hg->git project)]

Can you create a patch for esr52 and request uplift? Thanks.

Comment 41

[Jim Mathies [:jimm]]

(In reply to Jimmy from comment #38)
> Thanks for the information.
> 
> We did not realize that Firefox/Chrome call ShellexecuteExW in different
> ways. There is also a Windows bug in the path that firefox calls. We are
> working on a fix and will get this out in an early servicing patch.
> 
> Let me know if you require any other information.

Thanks for the follow up. Is there something we can change on our side? The code in question is here [1]. (In reply to Liz Henry (:lizzard) (needinfo? me) from comment #40)
> Can you create a patch for esr52 and request uplift? Thanks.

The existing patch should apply cleanly there, it's a one line change. I'll flag it.

Comment 42

[Jim Mathies [:jimm]]

Comment on attachment 8962342 [details] [diff] [review]
patch v.1

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
New security related behavioral change in Windows 10 needs to be enabled on our end through a minor code change.
User impact if declined: 
Issues with silent failures when handling file downloads under the newest version of windows 10. 
Fix Landed on Version:
 60, 61.
Risk to taking this patch (and alternatives if risky): 
Low risk, well understood change requested by Microsoft.
String or UUID changes made by this patch: 
none.

Comment 43

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8962342 [details] [diff] [review]
patch v.1

While this is a fix for a sec-moderate issue I think it warrants uplift to esr52 because it may be a widespread issue with the upcoming Windows update.

Comment 44

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/621034a42acc

Comment 45

[Ciprian Georgiu, Desktop QA]

Hi Jim,

I have retested this issue (followed the steps from comment 0) on Windows 10 x64 Version 1803 (OS Build 17134.5), and the prompts are still not displayed, on 52.8.1 Esr, 60.0.2 Release and 61.0b14 Beta. 

It seems that Edge/Chrome blocks properly both files mentioned in comment 0, when running them.

Any idea what might be wrong here, can you please take a look?

Comment 46

[Jimmy]

This fix required a change on the Windows side as well. It is going through servicing now and will be released at the end of this month. I'm not sure of the exact date.

I have validated that firefox has the same behavior as Edge/Chrome with this update.

Comment 47

[Jim Mathies [:jimm]]

(In reply to Jimmy from comment #46)
> This fix required a change on the Windows side as well. It is going through
> servicing now and will be released at the end of this month. I'm not sure of
> the exact date.
> 
> I have validated that firefox has the same behavior as Edge/Chrome with this
> update.

Thanks!

Comment 48

[Ciprian Georgiu, Desktop QA]

Closing this bug as verified fixed per comment 46.