1568003 - (CVE-2019-17019) On Windows, python files get executed instead of opened by notepad, because both Windows and the network suggest the file is text/plain, but ShellExecuteW will then open the file with python

Status: RESOLVED FIXED

(Firefox :: File Handling, defect, P1)

Description

[PRITHWISH KUMAR PAL]

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

Steps to reproduce:

Python need to be set in environment variables in client machine and in browser, in the application settings the content type as always ask.If some python code is there in google drive or office 365 and user clicks on download a pop up appears with 2 options - save file and open file. If user selects open file the python script will get executed. If the script is malicious it can perform malicious activity. Even it can connect remote server to download malicious file to file system.

Actual results:

Python script got executed by selecting open option.

Expected results:

Malicious code execution leading system compromise, change in OS level, delete of data etc.

Comment 1

[:Gijs (he/him)]

Dimi and Dan, do you think we should treat .py the same as .js, and/or if there's anything else we should do here (keeping in mind the fallout over jnlp)?

I'm inclined to say "no", given that .js is default-associated to the WSH and python isn't, and a python interpreter won't even be present by default. I also don't know a lot of people who OS-associate python files to the interpreter rather than the editor, but perhaps that's my bias... But I could be convinced, I guess?

Comment 2

[Daniel Veditz [:dveditz]]

I tried this on two different types of machines (Mac, Win). One had a default association of .py with a code editor and one didn't have an association at all. Other than installing development tools sufficient to build Firefox I didn't customize any handlers. On both systems Chrome made the same choice not to treat .py as special, and got the same OS-defined handler (or lack of). As you point out .js is a special danger on Windows because of defaults.

Commercial mass-market software isn't distributed in python form, but Business and home-grown enterprise stuff quite possibly is. I can see non-developer users of such software setting up the python runtime as the handler for .py files (or more likely the installer doing it) and then being more at risk.

Most Users: no python
Developers: python goes to editor
some users: python-based package installed, .py probably executes

How big is that 3rd group? Bigger than the second, surely, but enough so that it's worth inconveniencing the second group (who are likely more influential in terms of recommending Firefox)?

Comment 3

[PRITHWISH KUMAR PAL]

(In reply to :Gijs (back Aug 12; he/him) from comment #1)

Dimi and Dan, do you think we should treat .py the same as .js, and/or if there's anything else we should do here (keeping in mind the fallout over jnlp)?

I'm inclined to say "no", given that .js is default-associated to the WSH and python isn't, and a python interpreter won't even be present by default. I also don't know a lot of people who OS-associate python files to the interpreter rather than the editor, but perhaps that's my bias... But I could be convinced, I guess?

If attacker is aware of victim's machine work, the tool he uses then it may lead to a huge data breach or any specific attack.

Comment 4

[PRITHWISH KUMAR PAL]

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

I tried this on two different types of machines (Mac, Win). One had a default association of .py with a code editor and one didn't have an association at all. Other than installing development tools sufficient to build Firefox I didn't customize any handlers. On both systems Chrome made the same choice not to treat .py as special, and got the same OS-defined handler (or lack of). As you point out .js is a special danger on Windows because of defaults.

Commercial mass-market software isn't distributed in python form, but Business and home-grown enterprise stuff quite possibly is. I can see non-developer users of such software setting up the python runtime as the handler for .py files (or more likely the installer doing it) and then being more at risk.

Most Users: no python
Developers: python goes to editor
some users: python-based package installed, .py probably executes

How big is that 3rd group? Bigger than the second, surely, but enough so that it's worth inconveniencing the second group (who are likely more influential in terms of recommending Firefox)?

There is another category who are not developers. Instead they run online available python code in machines. They are the greatest victims.

Comment 5

[Dimi Lee [:dimi]]

Python script is included in the download protection binary list, so we may trigger dp ping after downloading, and the response may be roughly considered falling into one of these categories: safe, danger or unknown.
If I understand correctly, In Chrome, for certain extensions they think “may be dangerous”[1], when the response verdict type is unknown, they will show a warning message to ask users to confirm after downloading(I tried downloading some py files, but without any luck getting an “unknown” response to prove this). And for us, unknown are treated as safe for all the extensions.
I also checked the telemetry, a fairly large amount of python script dp ping responses are unknown, so this might be a way to consider to improve security if we think this is something worth doing.

[1] https://cs.chromium.org/chromium/src/chrome/browser/resources/safe_browsing/download_file_types.asciipb?l=806&rcl=9fcc7347a6466e7740e829f672ff19ae2b83ba9b

Comment 6

[PRITHWISH KUMAR PAL]

(In reply to Dimi Lee [OOO 7/13-8/2][:dimi][:dlee] from comment #5)

Python script is included in the download protection binary list, so we may trigger dp ping after downloading, and the response may be roughly considered falling into one of these categories: safe, danger or unknown.
If I understand correctly, In Chrome, for certain extensions they think “may be dangerous”[1], when the response verdict type is unknown, they will show a warning message to ask users to confirm after downloading(I tried downloading some py files, but without any luck getting an “unknown” response to prove this). And for us, unknown are treated as safe for all the extensions.
I also checked the telemetry, a fairly large amount of python script dp ping responses are unknown, so this might be a way to consider to improve security if we think this is something worth doing.

[1] https://cs.chromium.org/chromium/src/chrome/browser/resources/safe_browsing/download_file_types.asciipb?l=806&rcl=9fcc7347a6466e7740e829f672ff19ae2b83ba9

Please let me know whether I need to share any video to prove it that its working

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:Gijs, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 8

[:Gijs (he/him)]

Reporter: can you provide an online sample of (ie link to) a python file where we behave differently from Chrome on the same machine?

(In reply to Dimi Lee [OOO 7/13-8/2][:dimi][:dlee] from comment #5)

I also checked the telemetry, a fairly large amount of python script dp ping responses are unknown, so this might be a way to consider to improve security if we think this is something worth doing.

Only if it actually solves anything here, which I'm not sure it would without more details about whether we actually behave differently in the case that's being reported (and if so, if that's actually a result of the safebrowsing difference).

Comment 9

[PRITHWISH KUMAR PAL]

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

Reporter: can you provide an online sample of (ie link to) a python file where we behave differently from Chrome on the same machine?

(In reply to Dimi Lee [OOO 7/13-8/2][:dimi][:dlee] from comment #5)

I also checked the telemetry, a fairly large amount of python script dp ping responses are unknown, so this might be a way to consider to improve security if we think this is something worth doing.

Only if it actually solves anything here, which I'm not sure it would without more details about whether we actually behave differently in the case that's being reported (and if so, if that's actually a result of the safebrowsing difference).

If you want I can share a recorded version. Does bugzilla allow video upload?

Comment 10

[:Gijs (he/him)]

(In reply to PRITHWISH from comment #9)

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

Reporter: can you provide an online sample of (ie link to) a python file where we behave differently from Chrome on the same machine?

(In reply to Dimi Lee [OOO 7/13-8/2][:dimi][:dlee] from comment #5)

I also checked the telemetry, a fairly large amount of python script dp ping responses are unknown, so this might be a way to consider to improve security if we think this is something worth doing.

Only if it actually solves anything here, which I'm not sure it would without more details about whether we actually behave differently in the case that's being reported (and if so, if that's actually a result of the safebrowsing difference).

If you want I can share a recorded version. Does bugzilla allow video upload?

Yes, but there are filesize limits - depending on the size it may be easier to upload to youtube, dropbox or gdrive as an unlisted/private thing and share the link here.

That said, sharing a link to a broken python file where we behave differently from Chrome (on the same machine) would be the most helpful.

Comment 11

[PRITHWISH KUMAR PAL]

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

(In reply to PRITHWISH from comment #9)

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

Reporter: can you provide an online sample of (ie link to) a python file where we behave differently from Chrome on the same machine?

(In reply to Dimi Lee [OOO 7/13-8/2][:dimi][:dlee] from comment #5)

I also checked the telemetry, a fairly large amount of python script dp ping responses are unknown, so this might be a way to consider to improve security if we think this is something worth doing.

Only if it actually solves anything here, which I'm not sure it would without more details about whether we actually behave differently in the case that's being reported (and if so, if that's actually a result of the safebrowsing difference).

If you want I can share a recorded version. Does bugzilla allow video upload?

Yes, but there are filesize limits - depending on the size it may be easier to upload to youtube, dropbox or gdrive as an unlisted/private thing and share the link here.

That said, sharing a link to a broken python file where we behave differently from Chrome (on the same machine) would be the most helpful.

Within limited file size it is possible to this scripting attack.

Comment 12

[PRITHWISH KUMAR PAL]

It is a security vulnerability for firefox. Let me know what else need to be shared for this.

Comment 13

[:Gijs (he/him)]

(In reply to PRITHWISH from comment #12)

what else need to be shared for this.

Please provide a link to a python file with which this problem reproduces in Firefox and doesn't reproduce in Chrome.

Please also provide a screen recording of the Firefox vs. Chrome behaviour you're seeing for this file.

Comment 14

[PRITHWISH KUMAR PAL]

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

(In reply to PRITHWISH from comment #12)

what else need to be shared for this.

Please provide a link to a python file with which this problem reproduces in Firefox and doesn't reproduce in Chrome.

Please also provide a screen recording of the Firefox vs. Chrome behaviour you're seeing for this file.

https://drive.google.com/file/d/1gIR9YZDF1oQd0A3g_5sf1CXxO4p-iTuz/view?usp=sharing

Run it in firefox with about:preferences content type python as always ask

Comment 15

[:Gijs (he/him)]

(In reply to PRITHWISH from comment #14)

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

(In reply to PRITHWISH from comment #12)

what else need to be shared for this.

Please provide a link to a python file with which this problem reproduces in Firefox and doesn't reproduce in Chrome.

Please also provide a screen recording of the Firefox vs. Chrome behaviour you're seeing for this file.

https://drive.google.com/file/d/1gIR9YZDF1oQd0A3g_5sf1CXxO4p-iTuz/view?usp=sharing

Run it in firefox with about:preferences content type python as always ask

So what's different in Chrome? If you click the downloaded file in Chrome, doesn't exactly the same thing happen?

Comment 16

[PRITHWISH KUMAR PAL]

no

Comment 17

[Daniel Veditz [:dveditz]]

"no" is unhelpfully brief if it takes a week to get a response.

On Mac Chrome gives me the "This type of file can harm your computer. Do you want to keep ... anyway?" message when I click the download button on the drive page in comment 14. This is consistent with what Dimi said in comment 5, although at the time he was having trouble finding a file that would trigger it.

We know from the unhappiness over JNLP handling that our UX around dangerous types needs improving. If we have a bug or plan to improve that then we could/should throw python on the "dangerous" list once we've done that (have this bug depend on that one). Until that UX changes we can't. We certainly can't give "unknown" files the "known bad" safebrowsing treatment.

Comment 18

[PRITHWISH KUMAR PAL]

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

"no" is unhelpfully brief if it takes a week to get a response.

On Mac Chrome gives me the "This type of file can harm your computer. Do you want to keep ... anyway?" message when I click the download button on the drive page in comment 14. This is consistent with what Dimi said in comment 5, although at the time he was having trouble finding a file that would trigger it.

We know from the unhappiness over JNLP handling that our UX around dangerous types needs improving. If we have a bug or plan to improve that then we could/should throw python on the "dangerous" list once we've done that (have this bug depend on that one). Until that UX changes we can't. We certainly can't give "unknown" files the "known bad" safebrowsing treatment.

Apologize for late reply. The issue we are discussing about is regarding firefox not chrome. Firefox doesn't give any alert if python environment is set. It directly run the script in host machine.

Comment 19

[PRITHWISH KUMAR PAL]

(In reply to PRITHWISH from comment #18)

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

"no" is unhelpfully brief if it takes a week to get a response.

On Mac Chrome gives me the "This type of file can harm your computer. Do you want to keep ... anyway?" message when I click the download button on the drive page in comment 14. This is consistent with what Dimi said in comment 5, although at the time he was having trouble finding a file that would trigger it.

We know from the unhappiness over JNLP handling that our UX around dangerous types needs improving. If we have a bug or plan to improve that then we could/should throw python on the "dangerous" list once we've done that (have this bug depend on that one). Until that UX changes we can't. We certainly can't give "unknown" files the "known bad" safebrowsing treatment.

Apologize for late reply. The issue we are discussing about is regarding firefox not chrome. Firefox doesn't give any alert if python environment is set. It directly run the script in host machine.

Instead of enhancement it should be fixed so that it behaves properly as chrome i.e. when I click on open it will show the script in browser and will not execute it.

Comment 20

[Daniel Veditz [:dveditz]]

(In reply to PRITHWISH from comment #19)

Instead of enhancement it should be fixed so that it behaves properly as chrome i.e. when I click on open it will show the script in browser and will not execute it.

That's exactly what we see happen with your example link from comment 14. So the difference I'm seeing is our UI could make it clearer that this is a "dangerous" type, which would be an enhancement.

We're not seeing a "click and execute" behavior unless the user has at some point specified that python files are handled by an interpreter, and turned off the "always ask" setting. You have not provided a testcase that exhibits this behavior or even shared a video as requested showing this happen on your system from which we might be able to glean a few extra clues.

Comment 21

[Daniel Veditz [:dveditz]]

We already have bugs on improving the user experience around "dangerous type" files. So far we have seen no evidence that Python is uniquely dangerous since in none of the default configs we have found are they immediately executed.

Comment 22

[PRITHWISH KUMAR PAL]

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

We already have bugs on improving the user experience around "dangerous type" files. So far we have seen no evidence that Python is uniquely dangerous since in none of the default configs we have found are they immediately executed.

So according to you, this is not an issue? Python code with vulnerable code will not impact the individual user? Right?

Comment 23

[PRITHWISH KUMAR PAL]

I will like to public this issue and users decide whether its a vulnerability or not.

Comment 24

[PRITHWISH KUMAR PAL]

Will reopen this bug.

Comment 25

[:Gijs (he/him)]

The bug was closed incomplete because you've repeatedly refused to provide details of the specifics of this issue and why/how the behaviour is any different from other browsers.

Comment 26

[PRITHWISH KUMAR PAL]

I replied to each of the questions. Kindly follow the previous communications. Also let me know if any further information required. Keeping mind that after answering all the questions and clarifications the bug id was generated not before that.

Comment 27

[:Gijs (he/him)]

(In reply to PRITHWISH from comment #26)

I replied to each of the questions.

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

So what's different in Chrome?

You didn't answer this question. Please provide screen recordings of what you're doing in Chrome vs. Firefox, from a clean Firefox profile and default Chrome setup. It's still not clear what steps you're following, and why you think the effect in Firefox is different from the effect of the exact same steps in Chrome, to the point that it's a security bug.

Comment 28

[PRITHWISH KUMAR PAL]

Step 1: Set python in environment variable in pc and in browser the content type for python need to be set to Always ask
Step 2: Upload a malicious python script file in google drive / one drive etc. and share the link with victim
Step 3: Victim should try to access the url from Firefox and click on download
Step 4: When victim clicks on it a popup appears with two options - Save file and open with
Step 5: If victim selects open with the malicious script gets executed

While in other browsers, the malicious python script file will not run from browser. When victim clicks on download, the malicious file will get downloaded rather than getting executed.

Firefox is different from chrome in the sense that after clicking on download it again allow user to select option to save file or open with. Its a browser design flaw because when user clicks on download it is assumed that file need to be downloaded rather then asking for confirmation/permission. Moreover even if victim clicks on open it is executing the script but it should open the script as read only / editable mode using notepad or notepad++ rather than executing it.

Because of this feature there is a possibility that user may be a victim of remote code execution where code will be shared through social engineering means and when victim tries to open it the script will get executed leading to information disclosure, directory or file access or even file or directory data delete or encryption.

Comment 29

[:Gijs (he/him)]

(In reply to PRITHWISH from comment #28)

Step 1: Set python in environment variable in pc and in browser the content type for python need to be set to Always ask
Step 2: Upload a malicious python script file in google drive / one drive etc. and share the link with victim
Step 3: Victim should try to access the url from Firefox and click on download
Step 4: When victim clicks on it a popup appears with two options - Save file and open with
Step 5: If victim selects open with the malicious script gets executed

I can't reproduce, I have to manually find a python executable on disk if I want to open the file with python. Tested on macOS.

What OS are you testing on? What does step 1 ("set python in environment variable in pc") really mean? Are you pre-selecting python as the handler for .py files in Firefox? Or making it the OS default handler for python files? Or something else?

This is why a screencast would be helpful...

Comment 30

[PRITHWISH KUMAR PAL]

I am using windows 7. Its just an settings and configurations. I will be sharing a document. Let me know the process to share the video and documents.

Comment 31

[:Gijs (he/him)]

(In reply to PRITHWISH from comment #30)

I am using windows 7. Its just an settings and configurations.

There's really not enough detail here. In Firefox? In Windows? Both? And what did you set up?

I will be sharing a document. Let me know the process to share the video and documents.

Just attach them to the bug. If that doesn't work due to filesize, use a private file hosting site (e.g. box, dropbox, google drive, ...) of your choice and link the uploaded content here.

Comment 32

[PRITHWISH KUMAR PAL]

sure. I will do that and share the link

Comment 33

[PRITHWISH KUMAR PAL]

Please find the below link to download the steps for reproducing the vulnerability

https://drive.google.com/file/d/1_yGGtABf3uB1c9WKaSSh7rbto4Kg0zwF/view?usp=sharing

Comment 34

[:Gijs (he/him)]

OK, so finally it turns out there is a real issue here, and I'm sad that this wasn't picked up before - details matter when reporting bugs, so the python install + PATH modifications + win7 reference steps were crucial in figuring out what you're actually seeing.

When opening a python file served by gdrive, google sends us a content-disposition: attachment header, with a .py filename - but a text/plain mimetype.
So the "open with" option by default indicates the default OS handler for text/plain files... which is notepad.

Unfortunately, if the user selects "open", we tell the OS "open this file" (without directly specifying notepad), and then the user will find out that on Windows, the python installer made python the default handler for python scripts (and told the OS that python scripts' mimetypes are text/plain - I'm pretty sure there's helper app service code that does a sanity check that the extension and the mimetype match before relying on the mimetype).

None of this happens on Linux/mac.

Dan, I'm guessing we want to re-assess the security severity here. The UI is just lying here and we need to fix it. Perhaps you're still more likely to hit this accidentally than via malicious exploits, as the attacker would need to convince you to install python and update your PATH and all the rest of it, but even so.

:toshi/:bz/:mak, any ideas about what we should be doing instead? I can see a few options, but the file handling code is pretty old and crufty and I'm not sure how easily we can change basic assumptions here without breaking other uses. Options:

1. make the dialog / external app code pass in the actual handler instead of calling file.launch(). This will involve figuring out complete commandline invocations from the registry / windows APIs ourselves so it's a little messy. Also goes the opposite way from stated/suggested direction elsewhere ( https://bugzilla.mozilla.org/show_bug.cgi?id=1306338 ).
2. fix the external helper app service's concept of what the "default" is to more heavily favour the default for the file extension instead of the mimetype, even though file extensions are a stupid way to do this, and accept that this is just how Windows works.
3. add hacky checks for this specific case (or, slightly less hacky, for anything in the safebrowsing list of dodgy extensions)

TL;DR: too many things pretend python files are just "text/plain" - until they're not.

Comment 35

[PRITHWISH KUMAR PAL]

Kindly raise a cve number and share it with me.

Comment 36

[Toshihito Kikuchi [:toshi]]

I tried the repro steps locally. The problem I can see is Firefox executes a python script though Firefox's "Opening script_run_python.py" dialog says "Open with Notepad (default)".

:toshi/:bz/:mak, any ideas about what we should be doing instead?

I think the expected behavior should be either 1) to open with Notepad as shown in the dialog or 2) to make the dialog say "Open with Python.File (default)" keeping the current behavior. The latter idea is what we currently do for a link to a .py file without any Content-Disposition.

Interestingly, to get this repro, I didn't have to set any environment variables nor about:preference settings. I just installed Python for Windows with the default settings. I saw the same behavior on Win10, too.

Comment 37

[PRITHWISH KUMAR PAL]

Yes. because environment variables are set on installation by default and if not need to set manually

Comment 38

[Marco Bonardo [:mak]]

Provided it's not a solution to this bug, comment 5 sounds like something we may evaluate regardless in a separate bug. Did we discard that already? And if so, why?

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

So the "open with" option by default indicates the default OS handler for text/plain files... which is notepad.
Unfortunately, if the user selects "open", we tell the OS "open this file" (without directly specifying notepad), and then the user will find out that on Windows, the python installer made python the default handler for python scripts (and told the OS that python scripts' mimetypes are text/plain

2. fix the external helper app service's concept of what the "default" is to more heavily favour the default for the file extension instead of the mimetype, even though file extensions are a stupid way to do this, and accept that this is just how Windows works.

While it's not great, Windows always worked off file extensions, denying this fact seems to only bring troubles. I'd go this way. As :toshi pointed out it would be consistent with the non Content-Disposition case, so it seems the way to go.

Comment 39

[:Gijs (he/him)]

OK, I'll try to work out why we're doing something else in the content-disp case and if it's easy to make the content-disp case behave like the non-content-disp one.

Boris, still interested in hearing if you've got historical context here about things to watch out for, having only recently read some of e.g. bug 453455 and some of the issues around content-disp: attachment there.

Comment 40

[Boris Zbarsky [:bzbarsky]]

and told the OS that python scripts' mimetypes are text/plain

Yeah, this is just bonkers, as a general thing. As you said, we do verify that the extension of the file we're putting the data in is a valid extension for the MIME type, and fix the extension as needed if it's not, but if everyone lies to us about what they are doing... Like your TL;DR says, people really need to stop abusing the system. :(

fix the external helper app service's concept of what the "default" is to more heavily favour the default for the file extension instead of the mimetype, even though file extensions are a stupid way to do this, and accept that this is just how Windows works.

I think the code flow here is doing the "give me the info for this (type, extension) pair" thing, or at least I really hope it is. If the Windows implementation of the helper app service doesn't handle that correctly, then it needs to be fixed!

In particular, if we are given a type and an extension, and that extension is a valid extension for that type, and Windows does helper app lookup by extension, then we should be doing that lookup based on that extension.

What nsOSHelperAppService::GetMIMEInfoFromOS does right now is:

1. Get the default extension for the given type.
2. Look up the handler for that default extension.
3. If we found something, and the given extension is valid for the given type, add it to the MIME info.
4. If we found nothing for the default extension, look up info for the given extension.

or so (there are some more complications). It seems like the logic should be more like:

1. Look up the type for the given extension.
2. If this matches the given type, look up handlers by the given extension, since that's the extension we will end up using. We can use the existing nsOSHelperAppService::typeFromExtEquals thing on Windows for this, I bet.
3. Otherwise look up the default extension for the type and look up based on that.
4. If none of that found anything useful, continue to look up by the given extension, I guess, as now? I'm not sure what exactly should happen here if the given extension does not match the given MIME type in any sort of useful way and there is no handler for the default exension for the given MIME type.

What should get stored in our prefs in terms of how to handle things in the future, given all that, is an interesting question. we don't really want to associate the Python interpreter with all text/plain downloads, for example, but if everyone is lying to us about .py being text/plain, I don't see a simple way to avoid that short of a blacklist or something. Or persisting helper apps on a per (type, extension) basis, not a per-type basis, as long as the extension is a valid one for the type, etc.

Comment 41

[PRITHWISH KUMAR PAL]

Is there any further information required from my end? When can I expect the CVE number for the same?

Comment 42

[:Gijs (he/him)]

(In reply to Toshihito Kikuchi [:toshi] from comment #36)

I think the expected behavior should be either 1) to open with Notepad as shown in the dialog or 2) to make the dialog say "Open with Python.File (default)" keeping the current behavior. The latter idea is what we currently do for a link to a .py file without any Content-Disposition.

Can you provide an example? I sort of expect that the server you tested doesn't send text/plain as the mimetype, but it'd be good to have this confirmed.

Comment 43

[Toshihito Kikuchi [:toshi]]

Yes, in the case of 2), Content-Type was text/x-python.

Comment 44

[:Gijs (he/him)]

(In reply to Marco Bonardo [:mak] from comment #38)

Provided it's not a solution to this bug, comment 5 sounds like something we may evaluate regardless in a separate bug. Did we discard that already? And if so, why?

No, we should still look at this. Dimi, would you mind filing a separate, non-security bug for this, not mentioning any of the relevant details of this bug?

Comment 45

[Dimi Lee [:dimi]]

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

(In reply to Marco Bonardo [:mak] from comment #38)

Provided it's not a solution to this bug, comment 5 sounds like something we may evaluate regardless in a separate bug. Did we discard that already? And if so, why?

No, we should still look at this. Dimi, would you mind filing a separate, non-security bug for this, not mentioning any of the relevant details of this bug?

Done, Bug 1595729

Comment 46

[:Gijs (he/him)]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #40)

or so (there are some more complications). It seems like the logic should be more like:

1. Look up the type for the given extension.
2. If this matches the given type, look up handlers by the given extension, since that's the extension we will end up using. We can use the existing nsOSHelperAppService::typeFromExtEquals thing on Windows for this, I bet.
3. Otherwise look up the default extension for the type and look up based on that.
4. If none of that found anything useful, continue to look up by the given extension, I guess, as now? I'm not sure what exactly should happen here if the given extension does not match the given MIME type in any sort of useful way and there is no handler for the default exension for the given MIME type.

I think I've got a patch for this locally, but there's some remaining stuff to work out. FWIW, I think this is too risky a change to uplift, so we probably want to file a generic non-security "cover" bug to make it easier to deal with regressions and all the rest of this - or open up this bug.

If we want a patch for uplift it'd have to be something specific for the python+text/plain combination, but that feels very yucky so I'd prefer not to.

What should get stored in our prefs in terms of how to handle things in the future, given all that, is an interesting question. we don't really want to associate the Python interpreter with all text/plain downloads, for example, but if everyone is lying to us about .py being text/plain, I don't see a simple way to avoid that short of a blacklist or something. Or persisting helper apps on a per (type, extension) basis, not a per-type basis, as long as the extension is a valid one for the type, etc.

Right. I don't really want to change the stored types in the handler service only on Windows, and doing so everywhere would likely get us complaints from Linux users. Seems macOS is off in its own world with UTIs and whatnot, from a quick look. So not wild about this unless you think we really need to go this route?

If we're not doing that, we run into a conundrum: how to make an exception for python here to avoid permanent mis-association for text/plain. I see a few solutions:

1. blocklist / hardcode text/plain that doesn't use .txt as something for which you can't save the association. We already have a similar exception for application/octet-stream, so this isn't completely out of the question, but I suppose if you currently work with an enterprise system with (say) .dat files that it serves as text/plain, you're gonna be pretty peeved. A workaround would be saving the default with a file with the correct extension, see below.
2. blocklist / hardcode text/plain with any extensions on the ApplicationReputation list. This would stop permanently associating e.g. MS Office files mis-served as text/plain. Unsure how common that is (though worth noting that permanent association for any files mis-served as application/octet-stream, probably more common, is already not possible!). I guess we could perhaps avoid this by changing the mimetype based on the file extension? Though that's yet more risk in terms of change from the status quo, and would entail doublechecking how solid the logic for passing extensions is, ie do we end up passing .py for URLs like example.com/serve.py?file=foo ?
3. blocklist / hardcode text/plain with any extensions on the sExecutableExts list. This would avoid Office files but doesn't currently include python, so we'd need to add it, which probably has other side effects.
4. hardcode blocking text/plain just for python (.py/.pyc files). This feels too specific.
5. check the primary extension for a mimetype before allowing storing the choice permanently. This is tricky, because now we end up in the situation where some of the stated aims for the existing code (e.g. treating .jpg and .jpeg the same) won't work anymore - there can only be one primary extension for a given mimetype. For text/plain it'll be .txt, so we wouldn't offer the "always store" option for text/plain with a .py file, but we also wouldn't offer the "always store" option for at least one of .jpg or .jpeg with an image/jpeg mimetype.

Between all of these, I'm leaning towards 2, and accept that users can't change the default for bogus-mimetype MS office files.

In addition to that choice, as noted in (1) above, we should work out whether we should be honouring an existing saved "always open" choice for text/plain when receiving a text/plain file that happens to be a .py file (or anything else that doesn't have the "primary extension", .txt). I think the answer should probably be "yes"... in the hope that not many people have already selected they want all text/plain files to open with the python file handler... but I'm open to other ideas (like a one-time scrubbing of the association or whatever, or scrubbing it if it's python (which sort of points the finger at this bug and seems unlikely), or ...).

Comment 47

[:Gijs (he/him)]

Freddy, do you think we should file something with gdrive to suggest they don't serve python files with text/plain but use text/x-python (which is what macOS and, based on web searches, linux distros, normally use) ?

Mark, the internet suggests you worked on the Python Windows installer? Do you know why it registers as text/plain in the windows registry and what'd break if it used text/x-python like mac and Linux do? It looks like the registry stuff was added in https://bugs.python.org/issue23260 , in https://github.com/python/cpython/commit/bb24087a2cbfb186b540cc71a74ec8c39c1ebe3a , but I'm really not familiar with Python's infrastructure or community so unsure if there are more details somewhere and/or if this is even the right repository...

Comment 48

[Boris Zbarsky [:bzbarsky]]

I think this is too risky a change to uplift

I agree.

Seems macOS is off in its own world with UTIs and whatnot, from a quick look

Yeah, I'm not really sure how this works in practice. I've definitely had problems with us creating files like "ajax-175" that are actually PDFs and then MacOS refusing to open them as PDF...

Between all of these, I'm leaning towards 2, and accept that users can't change the default for bogus-mimetype MS office files.

I don't know that I have strong opinions here. I haven't stayed current on all the edge cases involved in this particular UX flow.

Comment 49

[Mark Hammond [:markh] [:mhammond]]

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

Mark, the internet suggests you worked on the Python Windows installer?

Not for many years now.

Do you know why it registers as text/plain in the windows registry and what'd break if it used text/x-python like mac and Linux do? It looks like the registry stuff was added in https://bugs.python.org/issue23260 , in https://github.com/python/cpython/commit/bb24087a2cbfb186b540cc71a74ec8c39c1ebe3a , but I'm really not familiar with Python's infrastructure or community so unsure if there are more details somewhere and/or if this is even the right repository...

That's the correct repo. The author of that patch actually works for MS, so I'm a little surprised to find it registered that way - but regardless, I suspect there's no good reason for it to be done that way and possibly just carrying forward some poor decisions made many years before. I'd be happy to open an issue on that repo, but it might be better to wait until this (or a similar) bug is made public? Even if Python does change, it's not going to change how we address this issue because we need to deal with already installed/released Python versions, right?

Comment 50

[:Gijs (he/him)]

(In reply to Mark Hammond [:markh] from comment #49)

Even if Python does change, it's not going to change how we address this issue because we need to deal with already installed/released Python versions, right?

Yep, just trying to figure out where all the moving pieces are here - but I definitely agree we should chase this after a Firefox fix is shipped.

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #48)

Between all of these, I'm leaning towards 2, and accept that users can't change the default for bogus-mimetype MS office files.

I don't know that I have strong opinions here. I haven't stayed current on all the edge cases involved in this particular UX flow.

OK. Marco, do you have thoughts on how best to detect we're handling .py as text/plain and not offer the "always do this" checkbox in the unknownContentType dialog on Windows in this case (see comment #46 for context) ?

Comment 51

[Frederik Braun [:freddy]]

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

Freddy, do you think we should file something with gdrive to suggest they don't serve python files with text/plain but use text/x-python (which is what macOS and, based on web searches, linux distros, normally use) ?

Yes, but imho we still ought to implement a patch that implements astop gap on our end.

Comment 52

[Marco Bonardo [:mak]]

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

OK. Marco, do you have thoughts on how best to detect we're handling .py as text/plain and not offer the "always do this" checkbox in the unknownContentType dialog on Windows in this case (see comment #46 for context) ?

First, I read again all of the discussion taking a bit more time, I think bz's proposal to change the handler search order makes sense, as well as your proposal to hardcode text/plain for things in the ApplicationReputation list.

For the handling, it's not very different from what we do for octet-stream here https://searchfox.org/mozilla-central/rev/e7c61f4a68b974d5fecd216dc7407b631a24eb8f/toolkit/mozapps/downloads/HelperAppDlg.jsm#512 so it sounds like we should be able to just add another case. We have both the url and the type there.
That will uncheck the checkbox and disable it. This UX choice is ugly, imo. The user doesn't know why this checkbox gets disabled, and the explanation is too complicate to show it. I think we should actually hide/collapse the checkbox and not show it, that is closer to a "we show you the feature only if the website supports it". It would be less frustrating for the user.

There is actually a difference from octet-stream, for which we never want to remember the action; here we may want to remember the action if the user changes the handler in the dialog to one valid for the extension (and its real type)... Unfortunately this would require the dialog to know all the handlers for the extension, and onchange check if the picked handler is in that list. That sounds over complicate and unnecessary.
In the end I'd just remove the checkbox and file Tech Evangelism bugs to ask servers to serve files with proper types...

If your question was wider, please let me know.

Comment 53

[:Gijs (he/him)]

(In reply to Frederik Braun [:freddyb] from comment #51)

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

Freddy, do you think we should file something with gdrive to suggest they don't serve python files with text/plain but use text/x-python (which is what macOS and, based on web searches, linux distros, normally use) ?

Yes, but imho we still ought to implement a patch that implements astop gap on our end.

For sure.

I'll shortly be putting up a patch to execute on the steps in comment #40 in bug 1597985. Please do not link the two bugs or comment on bug 1597985 with anything relating to this bug, python, or the vulnerability itself.

I'm planning to use either this bug or another separate bug to add checks to disable "always do this for files of this type" for files that are in the application reputation check list. comment #0 suggests this bug doesn't happen when "always ask" is not selected, and I'd like to understand that better first.

Comment 54

[PRITHWISH KUMAR PAL]

Hi All,

When can I expect the CVE number for the mentioned issue. It has been 4 months since the conversation is going on.

Comment 55

[:Gijs (he/him)]

(In reply to PRITHWISH from comment #54)

It has been 4 months since the conversation is going on.

It's only been 3 weeks since it became clear what issue you were actually reported, after it repeatedly took us a week or more to get answers to questions we asked you. We're actively working on a patch, see comment #53 (posted less than a week ago).

Comment 56

[:Gijs (he/him)]

(In reply to PRITHWISH from comment #54)

When can I expect the CVE number for the mentioned issue.

I don't think we normally assign CVEs before we release a version of Firefox that contains a fix for the issue. Dan, can you comment?

Comment 57

[PRITHWISH KUMAR PAL]

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

(In reply to PRITHWISH from comment #54)

When can I expect the CVE number for the mentioned issue.

I don't think we normally assign CVEs before we release a version of Firefox that contains a fix for the issue. Dan, can you comment?

But is a vulnerability.right?

Comment 58

[:Gijs (he/him)]

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

I'll shortly be putting up a patch to execute on the steps in comment #40 in bug 1597985. Please do not link the two bugs or comment on bug 1597985 with anything relating to this bug, python, or the vulnerability itself.

This seems to have landed and stuck without (so far) any negative consequences.

I'm planning to use either this bug or another separate bug to add checks to disable "always do this for files of this type" for files that are in the application reputation check list. comment #0 suggests this bug doesn't happen when "always ask" is not selected, and I'd like to understand that better first.

I'm still digging at this. I've verified that on existing 70/71 builds, if you pick "remember this choice" for the python file served as text plain, that correctly shows up as a permanent choice in about:preferences indicating you selected the OS default app for text/plain files, which it lists (correctly) as "Use Notepad (default)". Ditto for the new 72 nightly builds that have the patch for bug 1597985 - it's a little bit wrong, because the dialog shows "Open with python.exe", but the python.exe isn't what is remembered, it's just "use the default OS helper", with the mime type (of text/plain). Disabling the checkbox here would still reduce confusion, but probably isn't a security issue because there are no ill effects.

However, in both situations, we still prompt, even with the code as-is.

This could be because of bug 453455. However, even if I create an "adversarial" testcase that uses <a download href="foo.py">Link</a> and instruct the server to serve foo.py with Content-Type: text/plain without a Content-Disposition header, I already still get asked (ie the "always do this" gets ignored).

Still digging as to why this is. This is made a little bit trickier because I don't have a visual studio environment in the VM where I'm testing this so I can't easily step through what our compiled code is doing here. I'm trying to use the browser toolbox for this, but trying to break into HandlerService.js causes the debugged process to crash... So, still digging.

Comment 59

[Boris Zbarsky [:bzbarsky]]

and instruct the server to serve foo.py with Content-Type: text/plain without a Content-Disposition header

We implement the download attribute by effectively pretending that the channel had Content-Disposition: attachment. So you end up hitting the same code.

Comment 60

[:Gijs (he/him)]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #59)

and instruct the server to serve foo.py with Content-Type: text/plain without a Content-Disposition header

We implement the download attribute by effectively pretending that the channel had Content-Disposition: attachment. So you end up hitting the same code.

Is there any other way we could end up downloading something served as text/plain ?

Comment 61

[Boris Zbarsky [:bzbarsky]]

I think the answer is "no".

Comment 62

[:Gijs (he/him)]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #61)

I think the answer is "no".

OK, good, that's what I thought too.

That leaves 2 things, ish:

1. disable the checkbox in this case, even if the effects are benign from a security PoV
2. ensure we don't accidentally start launching python files with python if/when we fix bug 453455 (or if there's some case Boris and I are missing in terms of text/plain downloading that wouldn't allow for inline opening).

For (2), I'm leaning towards adding an explicit check for ApplicationReputation file extensions at https://searchfox.org/mozilla-central/rev/04d8e7629354bab9e6a285183e763410860c5006/uriloader/exthandler/nsExternalHelperAppService.cpp#1658-1663 , and fall back to the alwaysAsk case there, just as we do for the content disposition attachment case. Even though that's not necessary today for this specific case, it seems like it would be wise in terms of future-proofing.

Neither of these seem super urgent given the main fix of making the dialog reflect reality has landed and is now on beta, but I'll try and get it done this week or next so we can consider beta uplift.

Comment 63

[PRITHWISH KUMAR PAL]

Hi All,

Is the fix for the same has been published? It is opened since 5 months and neither I received the cve number or any acknowledgement for identifying the bug.

Comment 64

[:Gijs (he/him)]

(In reply to PRITHWISH from comment #63)

Is the fix for the same has been published?

The main portion of this fix (bug 1597985) is currently in beta (Firefox 72). It's not made it to the release channel yet; if all goes well that'll happen in January ( https://wiki.mozilla.org/RapidRelease/Calendar ).

It is opened since 5 months and neither I received the cve number or any acknowledgement for identifying the bug.

I already responded to your points about timelines in comment #55. It's been much less than 5 months since it actually became clear what the issue was - more like 5 weeks. As for a CVE, I don't think we assign one until the bug is fixed on our release channels. Dan can confirm whether I've understood this correctly.

This bug is still open because of the issues in comment #62. I was hoping to get to this sooner but various other things were dropped on my plate that took precedence. Because of end-of-year holidays, the other issues may miss 72 and not be released until 73.

Comment 65

[Daniel Veditz [:dveditz]]

(In reply to PRITHWISH from comment #63)

Is the fix for the same has been published? It is opened since 5 months and neither I received the cve number or any acknowledgement for identifying the bug.

As Gijs said we have not shipped this fix yet. The CVE and acknowledgement will go in the advisories we publish when we ship.

(In reply to :Gijs from comment #64)

This bug is still open because of the issues in comment #62. I was hoping to get to this sooner but various other things were dropped on my plate that took precedence. Because of end-of-year holidays, the other issues may miss 72 and not be released until 73.

As a not-yet-fixed bug this but will get missed by the advisory writing process. Can we split those additional issues into a follow-up bug, resolve this one FIXED, and then write an advisory for Fx72 on Jan 7?

Comment 66

[:Gijs (he/him)]

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

As a not-yet-fixed bug this but will get missed by the advisory writing process. Can we split those additional issues into a follow-up bug, resolve this one FIXED, and then write an advisory for Fx72 on Jan 7?

Sure. I filed bug 1605102.

Comment 67

[Ryan VanderMeulen [:RyanVM]]

Do we need to do something for ESR still?

Comment 68

[:Gijs (he/him)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #67)

Do we need to do something for ESR still?

I don't know. :-(

I don't think we can uplift bug 1597985 to ESR before we release 72. We need at least some confidence there aren't going to be major regressions, first. We could add code in 68 that's specific to the python or apprep/text/plain case, but I don't think we can realistically do that before we've shipped a fix on regular release.

Can we mark this tracking the 73 version of 68esr so we come back to it for that?

Comment 69

[Tom Ritter [:tjr]]

Attachment: advisory.txt

Comment 70

[Tom Ritter [:tjr]]

Attachment: advisory.txt

Comment 71

[PRITHWISH KUMAR PAL]

Hi,

Is the CVE-2019-17019 available in google? In the attached advisory.txt, my name has been mis spelled. It will be Prithwish Kumar Pal

Comment 72

[Mark Hammond [:markh] [:mhammond]]

ni? Tom for comment 71.

Comment 73

[PRITHWISH KUMAR PAL]

(In reply to PRITHWISH from comment #71)

Hi,

Is the CVE-2019-17019 available in google? In the attached advisory.txt, my name has been mis spelled. It will be Prithwish Kumar Pal

Hi Team,

In the mentioned CVE the reporter name has been wrongly spelled. It will be Prithwish Kumar Pal. Mis-spelled evidence is attached within.

Comment 74

[Tom Ritter [:tjr]]

I apologize for that - I filed https://github.com/mozilla/foundation-security-advisories/pull/40 to fix it on our website and we will submit the CVE details to MITRE shortly and that should have the correct spelling in it.

Comment 75

[Ryan VanderMeulen [:RyanVM]]

Is this ready for an ESR approval request?

Comment 76

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from bug 1597985 comment #4)

Comment on attachment 9110267 [details]
Bug 1597985 - prefer file extension as provided over default extension for mimetype to look up default applications on Windows, r?bzbarsky

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: sec-moderate
• User impact if declined: security issue
• Fix Landed on Version: 72
• Risk to taking this patch: Low-Medium
• Why is the change risky/not risky? (and alternatives if risky): we're changing longstanding behaviour of how we look up handler apps on Windows. Still, this has baked on 72 and we don't currently think there are any regressions - though it's a bit tricky given that there were some other changes to handler applications that muddy the waters (eg. bug 1601905).
• String or UUID changes made by this patch: nope

Ryan, I've put the form in here rather than the public bug? This is a bit messy... :-\

Comment 77

[:Gijs (he/him)]

A regression was just reported for bug 1597985, so I don't think this is ready for this ESR round. Leaving ni for Ryan to update tracking flags.

Comment 78

[Ryan VanderMeulen [:RyanVM]]

Pushing it back a cycle for now, but honestly, I'm thinking we should maybe just wontfix for ESR and let it go out with ESR78 instead.

Comment 79

[:Gijs (he/him)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #78)

Pushing it back a cycle for now, but honestly, I'm thinking we should maybe just wontfix for ESR and let it go out with ESR78 instead.

WFM. Does this mean you want to clear the tracking flag, too?

I'm also going to self-needinfo because ideally we should also contact the python installer folks about this, and maybe the gdrive folks.

Comment 80

[:Gijs (he/him)]

I've reported the python windows installer side of this to the python security mailing list.

I also spent a good half hour trying to find somewhere to report a non-security issue with gdrive, but was unable to find an issue tracker or other contact point (amusingly, https://support.google.com/drive/answer/4431192/ suggests there should be a contact option on the gdrive help page, but as far as I can tell this does not exist - the best thing I could find in the list on https://support.google.com/drive is "Contact Google Drive support" , which goes straight back to the first page). I eventually used "send feedback" on gdrive itself but my expectation is that nothing will happen as a result of that.

Comment 81

[:Gijs (he/him)]

The good folks at Python have updated their Windows installer in https://bugs.python.org/issue39631 . I'm not an expert on their release processes, but based on reading the changelog tea leaves, I think the fix should be in 3.7.8, and was landed on the main and 3.8 and 3.9 branches as well (but, afaict, there's been no release with the fix on those branches just yet, though it's in the 3.9.0 beta 2 release and later). I'm guessing this will not change existing installations, though I'm not 100% sure.