sys.path[0] security issues

@birkenfeld

BPO	16202
Nosy	@birkenfeld, @jaraco, @ncoghlan, @vstinner, @tiran, @benjaminp, @tarekziade, @jwilk, @merwok, @ericsnowcurrently, @hynek, @jdemeyer, @zooba, @aw1231
Files	fix_distutils.patch fix_distutils.patch sys_path_security.patch

^{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/merwok'
closed_at = None
created_at = <Date 2012-10-11.20:10:23.538>
labels = ['type-security', 'interpreter-core', '3.8', '3.9', '3.10', '3.7']
title = 'sys.path[0] security issues'
updated_at = <Date 2021-02-03.18:21:55.365>
user = 'https://github.com/jdemeyer'

bugs.python.org fields:

activity = <Date 2021-02-03.18:21:55.365>
actor = 'christian.heimes'
assignee = 'eric.araujo'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2012-10-11.20:10:23.538>
creator = 'jdemeyer'
dependencies = []
files = ['27542', '27543', '27923']
hgrepos = []
issue_num = 16202
keywords = ['patch']
message_count = 24.0
messages = ['172686', '172709', '172722', '172736', '172750', '172755', '172756', '172762', '172764', '172766', '172768', '172798', '172799', '172803', '172812', '172981', '172983', '172998', '173000', '175155', '275215', '275598', '311782', '386330']
nosy_count = 21.0
nosy_names = ['georg.brandl', 'jaraco', 'ncoghlan', 'vstinner', 'ThomasAH', 'christian.heimes', 'schmir', 'robertwb', 'benjamin.peterson', 'tarek', 'jwilk', 'eric.araujo', 'Arfrever', 'iankko', 'eric.snow', 'hynek', 'jdemeyer', 'steve.dower', 'Alan.Williams', 'vbraun', 'hasufell']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue16202'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8', 'Python 3.9', 'Python 3.10']

There is a serious security problem with Python's default sys.path. If I execute

$ python /tmp/somescript.py

then Python will add /tmp as sys.path[0], such that an "import foobar" will cause Python to read /tmp/foobar (or variations). This vulnerability exists in particular in distutils.util.byte_compile() with direct=False. Since the Python test suite calls this function, users running the Python test suite are vulnerable.

I think the root of this issue should be fixed: Python should not simply add stuff to sys.path without checking. In prepared a patch for CPython-2.7 which only adds sys.path[0] if it seems secure to do so, by looking at file/directory permissions and ownership. In particular, it would never add /tmp to sys.path, but it would still keep the current behaviour when running a script in a directory owned by the current user with 0755 permissions. See the patch for details.

I realize this goes against documented Python behaviour, but I think that a broken spec needs to be fixed. I also think that in most use cases, nothing is going to change because normally one doesn't need to import from /tmp. In any case, users can still manipulate sys.path directly.

Feel free to fix this issue in a different way than my patch, but I hope you at least implement the spirit of my patch. The patch has only been tested on Linux systems.

Credit goes to Volker Braun for first discovering this issue in Sage, see http://trac.sagemath.org/sage_trac/ticket/13579

Alternatively, one could fix distutils.util.byte_compile() to execute the script in safe, empty temp directory. Running scripts in /tmp remains, as it has always been, a bad idea.

Trying to determine if an import is "safe" can be arbitrarily complicated (e.g. what if the group-write bit is set, but you're the only member of that group, or there are special allow or deny ACLs for other users that aren't detected here). What notion of safeness belongs in the spec?

Robert: I don't think that running scripts in /tmp is inherently unsafe. It is Python's sys.path handling which makes it unsafe. That being said, I am not against distutils being "fixed" but I do think the root issue should be fixed.

And of course you're right about complicated permission checking and ACLs and what not. But I think my patch does the Right Thing in 99% of the cases, in particular for /tmp. I tried to err on the safe side.

The fact that Python's own testsuite tripped over this proves that this is subtle enough to merit some special handling.

It is not, and has never been, a good idea to run/compile anything off /tmp. This isn't specific to Python, it is just common sense that you don't hand over control of directory contents to others.
Removing /tmp from sys.path upon startup is not enough to guarantee safety. Many Python modules will happily add it back. Just as a random example, see profile.py: "sys.path.insert(0, os.path.dirname(progname))". The aim of the patch should be to warn the user of the dangers of running code in /tmp, not trying to make it safe (and, therefore, implicitly encouraging it).
The patch is too restrictive in my opinion, it rules out some plausible and perfectly safe use cases. For example, root owns directory and wheel owns Python script. Or sharing a group with a trusted user. Just disallowing o+w would be enough to save the unwary from executing in /tmp.

Robert Bradshaw's idea is the only feasible option for Python 2.7 or any other version except for 3.4dev. Your suggested modification to sys.path is out of option as it would create a backwards incompatibility with existing software.

I'm adding 2.6 to 3.4 as all versions of Python are affected.

Here's a fix to distutils. I think at least a warning is in order for running scripts from insecure directories, and ideally some happy medium can be found.

I'm all in favor for the proposal to add a warning when the script is in a world-writable directory. But any modification can't be added to stable version as it's a new feature.

Robert, you have to cleanup and remove the directory manually at the end of the block. mkdtemp() creates the directory but doesn't do house keeping.

If you don't plan any further Python-2 releases, it would be pity that this cannot be fixed. If you do plan a further Python-2 release, I find backwards compatibility a poor excuse. I'm not saying that backwards compatibility should be totally ignored, but it certainly should not trump everything either, especially for security issues. I carefully designed my patch to have no impact for most existing secure setups (but as I said, I'm open for improvements).

Ultimately it's Benjamin's and Georg's decision. They are the release managers of 2.7 to 3.3 and need to come to an agreement. You have to convince them that the proposed security restriction is worth the risk of breaking 3rd party software.

It would help if you describe the circumstances under which your patch doesn't add the module's path to sys.path.

disutils should definitely be fixed.

Good point about cleanup, patch updated.

I updated sys_path_security.patch by a newer version. This version will be merged in the Python package of Sage (http://www.sagemath.org/).

I realise that it looks unlikely that it will be merged in CPython, but at least it's here for reference.

What is the status of this issue? Is isolated mode (-I) a sufficient solution for you?

Reviewing the issue, I think there's still an open question regarding the way distutils handles generated script execution that may impact setuptools as, so adding Jason to the nosy list.

For the "don't set sys.path[0] by default" aspect, we would need a different executable that uses more paranoid defaults, which would be contingent on the PEP-432 startup refactoring landing for 3.7 (as too much behaviour is currently embedded inside Py_Main for alternate defaults to be reasonably maintained).

I just stumbled across this problem when starting "idle3" in a directory containing a copy of textwrap.py which was not compatible with python3.

In bpo-13506 idle3 was changed to behave like the regular python shell, i.e. as described here in this issue, and since my ~/.pythonrc.py imports readline, I could reproduce this problem by creating a bogus readline.py file and starting a python interpreter.

So besides being a security issue when starting python or idle in untrusted directories, it may cause technical issues when working in directories containing .py files with certain names.

Distutils is now deprecated (see PEP-632) and all tagged issues are being closed. From now until removal, only release blocking issues will be considered for distutils.

If this issue does not relate to distutils, please remove the component and reopen it. If you believe it still requires a fix, most likely the issue should be re-reported at https://github.com/pypa/setuptools

I think this can be closed since #57684 and #31542

Navigation Menu

Repository navigation

sys.path[0] security issues #60406

8 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

sys.path[0] security issues #60406

Description

Activity

jdemeyer commented on Oct 11, 2012

robertwb commented on Oct 11, 2012

jdemeyer commented on Oct 12, 2012

vbraun commented on Oct 12, 2012

tiran commented on Oct 12, 2012

robertwb commented on Oct 12, 2012

tiran commented on Oct 12, 2012

jdemeyer commented on Oct 12, 2012

tiran commented on Oct 12, 2012

benjaminp commented on Oct 12, 2012

robertwb commented on Oct 12, 2012

8 remaining items

jdemeyer commented on Nov 8, 2012

tiran commented on Sep 8, 2016

ncoghlan commented on Sep 10, 2016

ThomasAH commented on Feb 7, 2018

zooba commented on Feb 3, 2021

hauntsaninja commented on Jul 17, 2022

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions