Avoid adding an empty directory to sys.path when running a module with `-m`

[ztane]

BPO	33053
Nosy	@brettcannon, @ncoghlan, @vstinner, @jwilk, @ned-deily, @njsmith, @ericsnowcurrently, @eryksun, @ztane
PRs	bpo-33053: -m adds *starting* directory to sys.path #6231 bpo-33053: Remove test_cmd_line_script debugging print #6237 [3.7] bpo-33053: -m now adds *starting* directory to sys.path (GH-6231) #6236 [3.6] bpo-28655: Fix test_import.test_missing_source_legacy() #9589

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/ncoghlan'
closed_at = <Date 2018-03-25.13:50:03.376>
created_at = <Date 2018-03-12.11:57:14.902>
labels = ['interpreter-core', '3.8', 'type-feature', '3.7']
title = 'Avoid adding an empty directory to sys.path when running a module with `-m`'
updated_at = <Date 2018-09-26.16:09:42.678>
user = 'https://github.com/ztane'

bugs.python.org fields:

activity = <Date 2018-09-26.16:09:42.678>
actor = 'vstinner'
assignee = 'ncoghlan'
closed = True
closed_date = <Date 2018-03-25.13:50:03.376>
closer = 'ncoghlan'
components = ['Interpreter Core']
creation = <Date 2018-03-12.11:57:14.902>
creator = 'ztane'
dependencies = []
files = []
hgrepos = []
issue_num = 33053
keywords = ['patch']
message_count = 30.0
messages = ['313641', '313966', '314020', '314021', '314023', '314025', '314028', '314036', '314040', '314081', '314088', '314089', '314092', '314130', '314192', '314214', '314233', '314248', '314276', '314403', '314404', '314411', '314412', '314414', '314415', '314431', '314432', '315043', '315079', '326481']
nosy_count = 10.0
nosy_names = ['brett.cannon', 'ncoghlan', 'vstinner', 'jwilk', 'ned.deily', 'njs', 'eric.snow', 'eryksun', 'ztane', 'Viv']
pr_nums = ['6231', '6237', '6236', '9589']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue33053'
versions = ['Python 3.7', 'Python 3.8']

[ztane]

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 https://bugs.python.org/issue16202

I.e. whereas python3 /usr/lib/module.py 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.

[jwilk]

FWIW, this behavior is documented:

https://docs.python.org/3/using/cmdline.html#cmdoption-m
"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.

[ncoghlan]

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 script.py" 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')" > foo.py
    $ python3 -m foo
    Hello
    $ 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 __main__.py in sys.path[0] rather than searching the whole of sys.path (which is what currently happens).

[ncoghlan]

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.

[ncoghlan]

I've also separated out https://bugs.python.org/issue33095 (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.

[njsmith]

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 myscript.py'? Could we declare that the latter is the One Obvious Way and remove support for the former entirely?

[ncoghlan]

"python -m mypkg.myscript" does the right thing as far as local packages are concerned, whereas "python -m mypkg/myscript.py" will set you up for double-import bugs.

Note that you can almost always trigger arbitrary non-obvious code execution just by writing sitecustomize.py 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".

[jwilk]

-I implies -s, which is not something I want.

[ncoghlan]

https://bugs.python.org/issue13475 is the existing enhancement request to expose sys.path[0] management independently of the other execution isolation features.