Zipfile & directory execution in 3.5.4 also adds the parent directory to sys.path #76732
Labels
type-bug
An unexpected behavior, bug, or error
Comments
|
The issue that I reported in https://bugs.python.org/issue29723 is now affecting 3.5.4: |
|
(For clarity) The problem is that 3.5.4 adds the current directory to sys.path when running a subdirectory's __main__.py. No other version of Python does this. |
|
Unfortunately, it looks like bpo-29319 was backported to the 3.5 branch, but not the follow-up fix from bpo-29723: https://github.com/python/cpython/commits/3.5/Modules/main.c (The metadata on bpo-29319 indicated that the original change was targeted at 3.6+ only, and I didn't notice the message that mentioned the 3.5 branch, so I never even looked at 3.5 when working on bpo-29723 - I just assumed it wasn't affected) Adding unexpected directories to sys.path can definitely be a security problem, so I think the fix should be backported for 3.5.5, but I'm also wondering whether it might be a significant enough regression to warrant an extra "Oops, sorry, we broke it" binary release. (We don't have any good usage numbers on how often folks use directory execution vs other forms of execution, so we don't know how widespread any impact is likely to be) |
|
As a reminder: I'm currently scheduled to tag Python 3.5.5rc1 on January 21st, 2018, aka about six days from now. |
|
Nick makes this sound like it really should land in 3.5.5, so marking as a release blocker for now. |
|
PR submitted for 3.5. Since the problem was in a full release this time (rather than a pre-release the way it was for 3.6), I've reclassified it as a security bug, since it means some previously safe operations (where no user-writable directory would end up on sys.path even without the "-I" switch) are technically unsafe. There's a fair combination of factors required for it to actually cause a problem though:
That said, I suspect exactly the above may happen when using PEX files (https://github.com/pantsbuild/pex), since I can't find any reference in their code to forcing the use of isolated mode in the underlying interpreter. |
|
I've updated the issue title to reduce the need to have read bpo-29723 first to understand it. I've also filed pex-tool/pex#440 essentially asking the pex folks to check if they're affected. |
ncoghlan
changed the title
|
On Windows it's the directory that contains the zip file or directory with __main__.py, not the current directory. This seems normal to me. The directory or zip file is effectively executing as a script. I can understand wanting more isolated behavior in this case, i.e. make isolated mode the default when executing a directory or zip file as a script. |
|
Good catch Eryk, I misdiagnosed what was going on, since the current directory and the parent directory were the same location in Ned's particular example. I double checked, and we resolve symlinks in path entries *before* performing the incorrect directory traversal ("..." below indicates the usual standard path entries, "/tmp" is the unexpected entry introduced by the bug), so it isn't possible to use a symlink to get a user-controlled directory onto the path: That means that as far as I can tell, this is just a plain old bug, rather than a potential security concern (since privileged admin-controlled commands tend generally live in admin-controlled directories, as if they didn't, potential attackers would be able to replace them with arbitrary code directly) |
ncoghlan
changed the title
|
If you're certain it isn't a security bug, then please downgrade it from release blocker. I might permit a fix for it in 3.5.5 anyway, depending on how small it is, because nobody likes regressions. |
|
Thanks Larry - I've reduced the priority accordingly. While the NEWS entry sounds a bit complicated, the actual fix is nicely self-contained: it's all within Modules/main.c, and relates to how that manipulates sys.argv[0] during startup. Historically, the code would put a potentially incorrect entry into sys.path[0], then amend it later (and that amendment step could sometimes do the wrong thing), but after this patch, it stores the candidate entry in a local C variable and adds it to the path only after it knows it's correct to do so. The backport also comes with test cases to ensure everything's working as we expect, and this is the code that 3.6 has been using since its release. |
|
I can confirm that 3.5.5rc1 fixes the problem I had. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: