Title: Avoid adding an empty directory to sys.path when running a module with `-m`
Type: enhancement Stage:
Components: Interpreter Core Versions: Python 3.8
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: jwilk, ncoghlan, njs, ztane
Priority: normal Keywords:

Created on 2018-03-12 11:57 by ztane, last changed 2018-03-18 13:21 by ncoghlan.

Messages (9)
msg313641 - (view) Author: Antti Haapala (ztane) * Date: 2018-03-12 11:57
I think this is a really stupid security bug. Running a module with `-mmodule` seems to add '' as a path in sys.path, and in front. This is doubly wrong, because '' will stand for whatever the current working directory might happen to be at the time of the *subsequent import statements*, i.e. it is far worse than

I.e. whereas python3 /usr/lib/ wouldn't do that, python3 -mmodule would make it so that following a chdirs in code, imports would be executed from arbitrary locations. Verified on MacOS X, Ubuntu 17.10, using variety of Python versions up to 3.7.
msg313966 - (view) Author: Jakub Wilk (jwilk) Date: 2018-03-16 18:50
FWIW, this behavior is documented:
"As with the -c option, the current directory will be added to the start of sys.path."

With the -c option, at least you could easily remove the sys.path element yourself:

   python -c 'import sys; sys.path.remove(""); ...'

(This works, because sys is always a builtin module, so it won't be imported from cwd.)

I don't see any obvious way to make "python -m foo" secure in untrusted cwd, though.
The best I could come up with is:

   python -c 'import sys; sys.path.remove(""); import runpy; runpy._run_module_as_main("foo")'

which is quite insane.
msg314020 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2018-03-18 05:31
This isn't considered a security issue, as running "python3" interactively behaves in exactly the same way (i.e. tracking changes to the current working directory for the duration of the session), and running "python3" adds the full path to the current directory.

In all cases, the expectation is that end users will at least enable isolated mode if they don't want to risk importing arbitrary code from user controlled directories.

    $ echo "print('Hello')" >
    $ python3 -m foo
    $ python3 -Im foo
    /usr/bin/python3: No module named foo

However, I'm flagging this as an enhancement request for 3.8+ (with a reworded issue title), as the non-isolated `-m` switch algorithm for sys.path[0] calculation could be made more robust as follows:

1. Start out with "os.getcwd()" rather than the empty string
2. Once `__main__.__file__` has been calculated, delete sys.path[0] if __main__ was found somewhere else

A potentially related enhancement would be to modify directory & zipfile execution to only look for `` in `sys.path[0]` rather than searching the whole of `sys.path` (which is what currently happens).
msg314021 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2018-03-18 05:44
Also, a small upstream community interaction tip: if you want people to seriously consider your requests for changes in default behaviour (which inevitably risk backwards compatibility breaks), don't start out by insulting them.

Python's defaults are currently set up for a *trusted personal automation tool*, where the person writing the code is also the person running it.

By starting out with an insult like "I think this is a really stupid security bug", you're actually saying "I know very little about Python's history, or the audiences it was originally written to serve, and instead of politely suggesting an alternative behaviour that would be more robust in the face of system configuration errors, I'm going to try to use shame, guilt, and embarrassment to get people to do work for me". That kind of behaviour *isn't* a good way to get your issues addressed, but it *is* a good way to encourage people to decide that volunteering as an open source maintainer isn't worth the associated hassles.

The opening insult added nothing to your issue report, and could more productively have been replaced with an explanation of the expectations you had of the default behaviour, how you came by those expectations, and how the current behaviour failed to meet them.
msg314023 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2018-03-18 05:58
I've also separated out (a docs issue about making isolated mode more discoverable) based on the jwilk's comment that it wasn't clear how to disable the default "add the current directory to sys.path" behaviour.
msg314025 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-03-18 06:19
Whoa, wait, what?

I agree that the original post is not as diplomatic as it could be, but my reaction to learning about this just now is also shock and confusion, so I guess I can sympathize with the OP a bit...

The reason I'm surprised is that -- while this probably wasn't fully anticipated when -m was designed -- it's turned out to be a bit of a meme to replace calls like 'pip ...' with 'python -m pip ...', or 'virtualenv ...' with 'python -m virtualenv ...', etc. I thought these were generally pretty much equivalent. I definitely did *not* know that running 'python -m pip' could lead to executing arbitrary code from the cwd, and I'm sure I've run it inside e.g. random git checkouts. If someone had tried to spearphish me with this they would totally have succeeded. (I hope they haven't?)

If you want to run a file in the current directory, is there any advantage to doing 'python -m myscript' instead of 'python'? Could we declare that the latter is the One Obvious Way and remove support for the former entirely?
msg314028 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2018-03-18 07:04
"python -m mypkg.myscript" does the right thing as far as local packages are concerned, whereas "python -m mypkg/" will set you up for double-import bugs.

Note that you can almost always trigger arbitrary non-obvious code execution just by writing to the current directory, and any package you install can add a "<installation-site-packages>/arbitrary-code.pth" or "<user-site-packages>/arbitrary-code.pth" file that gets run at startup (setuptools has long relied on this to implement various features).

Opting in to isolated mode turns *all* of those features off by saying "I'm expecting to run system code only here, not custom user code".
msg314036 - (view) Author: Jakub Wilk (jwilk) Date: 2018-03-18 10:46
-I implies -s, which is not something I want.
msg314040 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2018-03-18 13:21 is the existing enhancement request to expose sys.path[0] management independently of the other execution isolation features.
Date User Action Args
2018-03-18 13:21:06ncoghlansetmessages: + msg314040
2018-03-18 10:46:07jwilksetmessages: + msg314036
2018-03-18 07:04:27ncoghlansetmessages: + msg314028
2018-03-18 06:19:42njssetnosy: + njs
messages: + msg314025
2018-03-18 05:58:43ncoghlansetmessages: + msg314023
2018-03-18 05:44:11ncoghlansetmessages: + msg314021
2018-03-18 05:31:20ncoghlansettype: security -> enhancement
title: Running a module with `-m` will add empty directory to sys.path -> Avoid adding an empty directory to sys.path when running a module with `-m`
messages: + msg314020
versions: + Python 3.8
2018-03-16 20:07:07ned.deilysetnosy: + ncoghlan
2018-03-16 18:50:11jwilksetnosy: + jwilk
messages: + msg313966
2018-03-12 11:57:15ztanecreate