Issue24534
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2015-06-29 19:30 by minrk, last changed 2022-04-11 14:58 by admin. This issue is now closed.
Files | ||||
---|---|---|---|---|
File name | Uploaded | Description | Edit | |
0001-disable-executing-code-in-.pth-files.patch | minrk, 2015-06-29 19:30 | disable executing code in .pth files | review |
Messages (15) | |||
---|---|---|---|
msg245959 - (view) | Author: Min RK (minrk) * | Date: 2015-06-29 19:30 | |
.pth files currently allow execution of arbitrary code, triggered by lines starting with `import`. This is a rarely understood, and often misbehaving feature. easy_install has used this feature to ensure that its packages are highest priority (even higher than stdlib). This is one of the unfortunate behaviors that pip undoes from easy_install, in part due to the problems it can cause. There is currently a proposal in setuptools to stop using this, even for easy_install. The attached patch removes support for executing code in .pth files, throwing an ImportWarning if any such attempts at import are seen. General question that might result in rejecting this patch: Are there any good/valid use cases for .pth files being able to execute arbitrary code at interpreter start time? If this is accepted, some implementation questions: 1. if the feature is removed in 3.6, should a DeprecationWarning be added to 3.5? 2. Is ImportWarning the right warning class (or should there even be a warning)? |
|||
msg245961 - (view) | Author: Tim Smith (tdsmith) * | Date: 2015-06-29 19:57 | |
In Homebrew we occasionally use .pth files to call site.addsitedir. This is useful when we want to add a directory to sys.path that contains .pth files that also need to be processed (for example, when adding a directory to sys.path that contains namespace packages). It would be helpful to retain a mechanism for adding site dirs from a .pth file. Homebrew also writes temporary probe .pths containing "import site; site.homebrew_was_here = True" in order to check whether .pth files are processed in a particular path, though we could equivalently write a temporary path to a .pth file and verify that it ends up in sys.path. Finally, Homebrew asks users to write .pth files with imports in some places where we could use usercustomize instead. |
|||
msg245964 - (view) | Author: R. David Murray (r.david.murray) * ![]() |
Date: 2015-06-29 20:18 | |
I'm guessing this feature has been around too long and is used in too many ways to remove. I believe it is effectively disabled by -I (if it is not, that would be a valid feature request). However, since setuptools was the pioneer in this area, if setuptools is no longer using it, perhaps a deprecation would be possible. I think it would probably have to happen over more than one release, to insure enough time to implement any features needed to replace the deprecated functionality. |
|||
msg245965 - (view) | Author: Marc-Andre Lemburg (lemburg) * ![]() |
Date: 2015-06-29 20:27 | |
On 29.06.2015 21:30, Min RK wrote: > > .pth files currently allow execution of arbitrary code, triggered by lines starting with `import`. This is a rarely understood, and often misbehaving feature. easy_install has used this feature to ensure that its packages are highest priority (even higher than stdlib). This is one of the unfortunate behaviors that pip undoes from easy_install, in part due to the problems it can cause. There is currently a proposal in setuptools to stop using this, even for easy_install. > > The attached patch removes support for executing code in .pth files, throwing an ImportWarning if any such attempts at import are seen. Such a change will require a PEP, since it's an essential feature that has been documented for a very long time: https://docs.python.org/3.5/library/site.html and is used by a lot of existing setuptools installations, which would break if Python were to remove support for this. The PEP would also need to address the reasons for removing the feature, e.g. explain possible attack vectors, confusion caused by this, etc. You can then reference this patch in the PEP. Thanks, -- Marc-Andre Lemburg eGenix.com |
|||
msg245967 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2015-06-29 20:41 | |
> The attached patch removes support for executing code in .pth files This change will basically break all Python applications. Don't do that. If you believe that we can smoothly move to a world without .pth files, you should propose an overall plan, step by step. It implies to add an option to Python to disable .pth files. It might be enabled by the -I command line option. https://docs.python.org/dev/using/cmdline.html#cmdoption-I |
|||
msg245983 - (view) | Author: Nick Coghlan (ncoghlan) * ![]() |
Date: 2015-06-30 08:20 | |
As others have noted, we're not going to change this default in the standard CPython executable (due to the degree of disruption involved), and the -S and -I switches already effectively turn it off (by disabling site module processing entirely) However, it could be reasonable to consider offering a way to disable *just* the execution of arbitrary code from .pth files, while otherwise allowing the site module to run. |
|||
msg245996 - (view) | Author: Min RK (minrk) * | Date: 2015-06-30 18:52 | |
Thanks for the feedback, I thought it might be a long shot. I will go back to removing the *use* of the feature everywhere I can find it, since it is so problematic and rarely, if ever, desirable. > it's an essential feature that has been documented for a very long time > https://docs.python.org/3.5/library/site.html The entirety of the documentation of this feature appears to be this sentence on that page: > Lines starting with import (followed by space or tab) are executed. No explanation or examples are given, nor any reasoning about the feature or why one might use it. > This change will basically break all Python applications This surprises me. Can you elaborate? I have not seen an application rely on executing code in .pth files. > If you believe that we can smoothly move to a world without .pth files, > you should propose an overall plan, step by step. I have no desire to remove .pth files. .pth files are a fine way to add locations to sys.path. It's .pth files *executing arbitrary code* that's the problem, very surprising, and a source of many errors (especially how it is used in setuptools). |
|||
msg245998 - (view) | Author: Marc-Andre Lemburg (lemburg) * ![]() |
Date: 2015-06-30 20:36 | |
On 30.06.2015 20:52, Min RK wrote: > > Thanks for the feedback, I thought it might be a long shot. I will go back to removing the *use* of the feature everywhere I can find it, since it is so problematic and rarely, if ever, desirable. Could you please post an example of where the feature is problematic ? >> it's an essential feature that has been documented for a very long time >> https://docs.python.org/3.5/library/site.html > > The entirety of the documentation of this feature appears to be this sentence on that page: > >> Lines starting with import (followed by space or tab) are executed. Yep, but it's enough to be in active use. |
|||
msg245999 - (view) | Author: Min RK (minrk) * | Date: 2015-06-30 20:49 | |
> Could you please post an example of where the feature is problematic ? setuptools/easy_install is the major one, which effectively does `sys.path[:0] = pth_contents`, breaking import priority. This has been known to result in adding `/usr/lib/pythonX.Y/dist-packages` to the front of sys.path, having higher priority that the stdlib or `--user` -installed packages (I helped a user deal with a completely broken installation that was a result of exactly this last week). The result can often be that `pip list` doesn't accurately describe the versions of packages that are imported. It also causes `pip install -e` to result in completely different import priority from `pip install`, which doesn't use easy-install.pth. Removing the code execution from `easy-install.pth` solves all of these problems. |
|||
msg246000 - (view) | Author: Marc-Andre Lemburg (lemburg) * ![]() |
Date: 2015-06-30 21:15 | |
On 30.06.2015 22:49, Min RK wrote: > >> Could you please post an example of where the feature is problematic ? > > setuptools/easy_install is the major one, which effectively does `sys.path[:0] = pth_contents`, breaking import priority. This has been known to result in adding `/usr/lib/pythonX.Y/dist-packages` to the front of sys.path, having higher priority that the stdlib or `--user` -installed packages (I helped a user deal with a completely broken installation that was a result of exactly this last week). The result can often be that `pip list` doesn't accurately describe the versions of packages that are imported. It also causes `pip install -e` to result in completely different import priority from `pip install`, which doesn't use easy-install.pth. Removing the code execution from `easy-install.pth` solves all of these problems. Ok, so you regard the way that setuptools uses the feature as problematic. I'm sure some people will agree (including myself), but this is not a good reason to remove the feature altogether. Just because a feature can be misused doesn't make it a bad feature. Otherwise, we'd have quite a few things we'd have to remove from Python :-) I'd suggest to try to fix the setuptools uses of the feature instead - in a way that doesn't break all setuptools installations, of course. Perhaps you could submit a fix for this to the setuptools maintainers instead. Thanks, -- Marc-Andre Lemburg eGenix.com |
|||
msg246003 - (view) | Author: Min RK (minrk) * | Date: 2015-06-30 22:16 | |
> Just because a feature can be misused doesn't make it a bad feature. That's fair. I'm just not aware of any uses of this feature that aren't misuses, hence the patch. > Perhaps you could submit a fix for this to the setuptools maintainers instead. Yes, that's definite the right thing to do, and in fact the first thing I did. It looks like that patch is likely to be merged; it is certainly much less disruptive. That's where I started, then I decided to bring it up to Python itself after reading up on the exploited feature, as it seemed to me like a feature with no use other than misuse. Thanks for your time. |
|||
msg246005 - (view) | Author: Marc-Andre Lemburg (lemburg) * ![]() |
Date: 2015-06-30 22:30 | |
On 01.07.2015 00:16, Min RK wrote: > >> Just because a feature can be misused doesn't make it a bad feature. > > That's fair. I'm just not aware of any uses of this feature that aren't misuses, hence the patch. I don't remember the details of why this feature was added, but can imagine that it was supposed to enable installation of new importers via .pth files. Without this feature it would not be possible to add entries to sys.path via .pth files which can only be handled by non-standard importers. >> Perhaps you could submit a fix for this to the setuptools maintainers instead. > > Yes, that's definite the right thing to do, and in fact the first thing I did. It looks like that patch is likely to be merged; it is certainly much less disruptive. That's where I started, then I decided to bring it up to Python itself after reading up on the exploited feature, as it seemed to me like a feature with no use other than misuse. Thanks, that's certainly a good way forward :-) |
|||
msg246076 - (view) | Author: Paul Moore (paul.moore) * ![]() |
Date: 2015-07-02 13:51 | |
On 30 June 2015 at 23:30, M.-A. Lemburg <mal@egenix.com> wrote: > I don't remember the details of why this feature was added, > but can imagine that it was supposed to enable installation > of new importers via .pth files. I don't know for certain if this feature was already available when importers were added, but there was certainly never any thought of adding importers via .pth files when we designed them. We expected importers to be added to sys.(meta_)path by explicit code run at application startup, before the imports that needed it. I'm glad to hear that setuptools is reconsidering its import priority hacking. And I also have never seen any use of executable code in .pth files that wasn't at best a dubious hack. But removing the feature (as opposed to getting packages to stop misusing the feature) is too much of a compatibility break. It might be worth putting together a documentation patch that expands what the docs say about the feature. Pointing out that it's easy to misuse, and advising caution, might be a reasonable thing to do. (Adding an example of a reasonable use of the feature would be even better, but that would require thinking of a reasonable use!!! :-)) |
|||
msg246082 - (view) | Author: Eric Snow (eric.snow) * ![]() |
Date: 2015-07-02 14:59 | |
Note that the idea of replacing .pth files came up a couple years ago: https://mail.python.org/pipermail/import-sig/2013-July/000645.html That proposal didn't go anywhere basically because there were more important things to work on. :) |
|||
msg246087 - (view) | Author: Eric Snow (eric.snow) * ![]() |
Date: 2015-07-02 16:18 | |
FYI, support for .pth has been around since at least Python 2.0. However, support for imports in .pth files was added in 2.1: changeset: 15815:868d2acf745808c9033f57cd5829d97a69ecf56e branch: legacy-trunk user: Martin v. Löwis <martin@v.loewis.de> date: Thu Jan 11 13:02:43 2001 +0000 summary: Patch #103134: Support import lines in pth files. From what I understand .pth file weren't meant to be used in the way they are. Some folks are just really good at exploiting unintended behaviors based on an implementation. :) We've been stuck with the "feature" ever since. In this case the implementation happened to be a bit lax in how it evaluated each line. It should have been strict about allowing only single import statements. Instead it just made sure the line started with "import" and then exec'ed it. A check for ";" would have been sufficient. That said, I don't fault Martin (or whoever actually wrote it) at all. The implementation doesn't really bother me. Sure, it could have been more careful, but honestly how could anyone be expected to anticipate the consequence here. Ultimately, folks that looked closely enough at the source to figure out the hack would have had enough context to know the intent. They should have opened a bug report rather that take advantage of the loophole. If their need was sufficient they could have easily proposed an explicit mechanism to get what they wanted. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-04-11 14:58:18 | admin | set | github: 68722 |
2015-07-02 16:18:34 | eric.snow | set | messages: + msg246087 |
2015-07-02 14:59:12 | eric.snow | set | messages: + msg246082 |
2015-07-02 13:51:09 | paul.moore | set | nosy:
+ paul.moore messages: + msg246076 |
2015-06-30 22:30:19 | lemburg | set | messages: + msg246005 |
2015-06-30 22:16:18 | minrk | set | messages: + msg246003 |
2015-06-30 21:15:27 | lemburg | set | messages: + msg246000 |
2015-06-30 20:49:45 | minrk | set | messages: + msg245999 |
2015-06-30 20:36:24 | lemburg | set | messages: + msg245998 |
2015-06-30 18:52:18 | minrk | set | status: open -> closed resolution: rejected messages: + msg245996 |
2015-06-30 08:42:20 | christian.heimes | set | nosy:
+ christian.heimes |
2015-06-30 08:20:54 | ncoghlan | set | messages: + msg245983 |
2015-06-29 20:41:57 | vstinner | set | nosy:
+ vstinner messages: + msg245967 |
2015-06-29 20:27:16 | lemburg | set | nosy:
+ lemburg messages: + msg245965 |
2015-06-29 20:18:47 | r.david.murray | set | nosy:
+ r.david.murray messages: + msg245964 |
2015-06-29 19:57:36 | tdsmith | set | nosy:
+ tdsmith messages: + msg245961 |
2015-06-29 19:49:45 | ned.deily | set | nosy:
+ brett.cannon, ncoghlan, eric.snow |
2015-06-29 19:30:39 | minrk | create |