classification
Title: Readline module loading in interactive mode
Type: security Stage: needs patch
Components: Interpreter Core Versions: Python 3.6, Python 3.5, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder: Don't import readline in isolated mode
View: 28192
Assigned To: Nosy List: Niels.Heinen, christian.heimes, eric.araujo, eric.snow, jcea, loewis, ncoghlan, pitrou, r.david.murray, serhiy.storchaka, steve.dower, vstinner, yaccz
Priority: normal Keywords:

Created on 2011-06-02 11:57 by Niels.Heinen, last changed 2020-03-06 20:51 by brett.cannon.

Messages (17)
msg137473 - (view) Author: Niels Heinen (Niels.Heinen) Date: 2011-06-02 11:57
Running the python binary without a script or using the -i flag will
start the process in interactive mode. The interactive mode requires an
external module to be loaded: readline.

Per default behavior, Python also tries to load this module from the current working directory (see also trace below)

strcpy(0x7fff17609ed8, ".so")                  = 0x7fff17609ed8
fopen64("readline.so", "rb" <unfinished ...>
SYS_open("readline.so", 0, 0666)               = -2
<... fopen64 resumed> )                        = 0
strcpy(0x7fff17609ed8, "module.so")            = 0x7fff17609ed8
fopen64("readlinemodule.so", "rb" <unfinished ...>
SYS_open("readlinemodule.so", 0, 0666)

The module is imported in Modules/main.c line 663:

  if ((Py_InspectFlag || ......
    isatty(fileno(stdin))) {
      PyObject *v;
      v = PyImport_ImportModule("readline");


Why consider this a security bug: basically because you don't expect a
program to import a shared library from your current directory _unless_
you explicitly tell it to (e.g. import blah).

On a multi user system, someone could plant a malicious shared libraries
named "readline.so" in an attempt to hack a user that runs python in
interactive mode.

The risk obviously _very_ low but nevertheless worth to consider improving by, for example, loading readline with a more strict path? (e.g.  python lib directories only?)

Niels



AN EXAMPLE:
-----------
The code below is compiled to readline.so and stored in /tmp:

  void __attribute__ ((constructor)) _load();
  void _load() {
      printf("DING DONG!\n");

  }

foo@foo:/tmp$ ls -l /tmp/readline.so 
-rwxr-x--- 1 nnnnn nnn 7952 Mar 29 16:24 /tmp/readline.so
foo@foo:/tmp$ python
Python 2.6.5 (r265:79063, Apr 16 2010, 13:57:41) 
[GCC 4.4.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
DING DONG!
>>>
msg137475 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-06-02 12:38
This is a general principle of how Python runs in interactive mode and is not confined to loading readline.  The same would be true for any module loaded during startup, and there are quite a few that are so loaded.  Since loading modules from the current working directory is an important feature of using python in interactive mode, this is not something that is likely to be changed.
msg137529 - (view) Author: √Čric Araujo (eric.araujo) * (Python committer) Date: 2011-06-03 15:42
+1 to what David said.  See also #5753.
msg137741 - (view) Author: Niels Heinen (Niels.Heinen) Date: 2011-06-06 15:09
Hi Eric, David,

This means that you cannot type "python" and press <enter> in any shared directory without the risk of a malicious readlinemodule.so being imported and executed.  

I think this is different from a scenario where someone explicitly runs a script or imports a module in interactive mode where it is also reasonable that such a person understands the importing mechanism.

Thanks for the quick responses btw!

Niels
msg137752 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-06-06 16:58
I've done a little poking around, and it looks like you are correct and I'm wrong. It appears that readline.so is or should be a special case.  I've added some people to nosy to see what they think.

Specifically, it appears that if I put a file that should shadow a library module that is imported at python startup time (eg: os.py) into my current working directory I still get the os.py from the appropriate lib directory, even though '' is first in my sys.path.  This is not how I thought it worked, but it is my observation.  I tested this on 2.6.6, 2.7.1 and 3.3 tip.
msg137804 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-06-07 11:08
I don't think readline is "special-cased":

$ echo "1/0" > logging.py
$ cpython/default/python
Python 3.3a0 (default:d8502fee4638+, Jun  6 2011, 19:13:58) 
[GCC 4.4.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import logging
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "logging.py", line 1, in <module>
    1/0
ZeroDivisionError: division by zero
msg137821 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-06-07 13:13
Python 3.3a0 (default:7323a865457a+, Jun  5 2011, 19:22:38) 
[GCC 4.5.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.modules['logging']
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
KeyError: 'logging'
>>> sys.modules['os']
<module 'os' from '/home/rdmurray/python/p33/Lib/os.py'>

The difference is that logging is not imported at startup. So, however os (and friends, there are a lot of modules in sys.modules at startup) is imported, it is different from how readline.so is imported.
msg137824 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-06-07 13:21
> The difference is that logging is not imported at startup. So, however
> os (and friends, there are a lot of modules in sys.modules at startup)
> is imported, it is different from how readline.so is imported.

For the record, os is imported by the _io module:

    /* put os in the module state */
    state->os_module = PyImport_ImportModule("os");
    if (state->os_module == NULL)
        goto fail;

(in Modules/_io/_iomodule.c)

This probably happens before sys.path is
adjusted/tweaked/fixed/garbled/whatever.
msg137828 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-06-07 14:28
Yeah, that would be my guess.  And readline.so is imported in main at a point where it has decided we are going into interactive mode, which is presumably after all other initialization has taken place, including the path munging.

Thus my suggestion that that particular import of readline.so should be special cased...
msg173544 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-10-22 16:49
This issue was fixed in 3.3, but not in 2.7 or 3.2.

$ strace ./python -i </dev/null 2>&1 | grep readline
stat64("/home/serhiy/py/cpython3.3/build/lib.linux-i686-3.3/readline.cpython-33m.so", {st_mode=S_IFREG|0755, st_size=52511, ...}) = 0
open("/home/serhiy/py/cpython3.3/build/lib.linux-i686-3.3/readline.cpython-33m.so", O_RDONLY) = 4
open("/lib/libreadline.so.6", O_RDONLY) = 4
msg173551 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-10-22 18:23
Serhiy, I don't think it's fixed in 3.3, or perhaps I'm misunderstanding what you mean by "fixed". If you create readline.cpython-33m.so in your cwd and then run python, the fake readline will still be loaded instead of the real one.

For example (here with 3.4):

$ touch readline.cpython-34dm.so
$ ./python
Python 3.4.0a0 (default:2a0c9472c89c, Oct 21 2012, 23:24:06) 
[GCC 4.5.2] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import readline
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: ./readline.cpython-34dm.so: file too short
msg173552 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-10-22 18:27
Regardless, I'm not sure what we should do about this issue. Loading readline is obviously provided as a convenience to make the interpreter prompt easier to use. Several of us would probably like to go a bit further and also add tab-completion (see issue5845).

It stands that while the -S and -E option allow to disable any customization a user might have done (which is necessary for e.g. suid scripts), the automatic insertion of '' into sys.path has no way of being undone.
msg173561 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-10-22 19:10
I understand what happens. Python 3.3+ uses getdents(), not stat() for module search. Therefore stat() is not called for non-existed files.

> It stands that while the -S and -E option allow to disable any customization a user might have done (which is necessary for e.g. suid scripts), the automatic insertion of '' into sys.path has no way of being undone.

Python used not only for suid scripts.
msg187597 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-04-22 22:42
We may add a command line option and/or an environment variable to not add the current directory to sys.path.

Changing the current behaviour may break many applications / use cases.
msg187598 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-04-22 22:46
> We may add a command line option and/or an environment variable to not add the current directory to sys.path.

Oh, this is exactly what the issue #16499 proposes.
msg252029 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-10-01 14:05
This issue was reported again in issue 25288.

To summarize: the cwd should only be used for imports *after* the command prompt is displayed, and readline is imported *before* the prompt is displayed but currently is imported from the cwd.  This should be fixed.
msg277330 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2016-09-24 19:22
Steve took care of the readline import for isolated mode in #28192. We can't change the default behavior. If you want to prevent Python from important files from either cwd, user packages or env vars, you have to use isolated mode. System scripts should use the isolated mode flag, too.
History
Date User Action Args
2020-03-06 20:51:30brett.cannonsetstatus: pending -> open
nosy: - brett.cannon
2016-09-24 19:22:02christian.heimessetstatus: open -> pending

nosy: + christian.heimes, steve.dower
messages: + msg277330

superseder: Don't import readline in isolated mode
2015-10-01 23:29:55eric.snowsetnosy: + eric.snow
2015-10-01 14:05:34r.david.murraysetmessages: + msg252029
2015-10-01 14:03:50r.david.murraysetstage: needs patch
versions: + Python 3.5, Python 3.6, - Python 3.2, Python 3.3, Python 3.4
2015-10-01 14:03:10r.david.murraylinkissue25288 superseder
2013-04-24 11:43:33yacczsetnosy: + yaccz
2013-04-22 22:46:17vstinnersetmessages: + msg187598
2013-04-22 22:42:13vstinnersetmessages: + msg187597
2012-10-22 19:10:03serhiy.storchakasetmessages: + msg173561
2012-10-22 18:27:59pitrousetnosy: + loewis
messages: + msg173552
2012-10-22 18:23:56pitrousetmessages: + msg173551
versions: + Python 3.3, Python 3.4
2012-10-22 16:49:57serhiy.storchakasetversions: - Python 2.6, Python 3.1, Python 3.3
nosy: + serhiy.storchaka, ncoghlan

messages: + msg173544

components: + Interpreter Core
2011-06-07 14:28:12r.david.murraysetmessages: + msg137828
2011-06-07 13:21:53pitrousetmessages: + msg137824
2011-06-07 13:13:00r.david.murraysetmessages: + msg137821
2011-06-07 11:08:03pitrousetmessages: + msg137804
2011-06-06 16:58:41r.david.murraysetnosy: + brett.cannon, pitrou, vstinner

messages: + msg137752
versions: + Python 3.1, Python 2.7, Python 3.2, Python 3.3
2011-06-06 15:09:15Niels.Heinensetmessages: + msg137741
2011-06-03 15:42:37eric.araujosetnosy: + eric.araujo
messages: + msg137529
2011-06-02 15:38:18jceasetnosy: + jcea
2011-06-02 12:38:20r.david.murraysetnosy: + r.david.murray
messages: + msg137475
2011-06-02 11:57:39Niels.Heinencreate