Issue22689
Created on 2014-10-21 21:03 by aidanhs, last changed 2022-04-11 14:58 by admin. This issue is now closed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 8194 | merged | serhiy.storchaka, 2018-07-09 09:53 | |
| PR 8206 | merged | miss-islington, 2018-07-09 18:47 | |
| Messages (5) | |||
|---|---|---|---|
| msg229788 - (view) | Author: Aidan Hobson Sayers (aidanhs) | Date: 2014-10-21 21:03 | |
Posix says the following on the subject of getenv: > The returned string pointer might be invalidated or the string content might be overwritten by a subsequent call to getenv() (http://pubs.opengroup.org/onlinepubs/9699919799/functions/getenv.html) Unfortunately, in Modules/getpath.c: static void calculate_path(void) { [...] char *_rtpypath = Py_GETENV("PYTHONPATH"); /* XXX use wide version on Windows */ wchar_t *rtpypath = NULL; wchar_t *home = Py_GetPythonHome(); char *_path = getenv("PATH"); So 3 potential getenv calls in quick succession, meaning _rtpypath and home can become junk before they get used and Python crashes before it can start up (it becomes unable to find the site module). Unfortunately it looks like the assumption that getenv pointers will remain safe forever is used in a few places in python. Explicit notes on the correct use of getenv: https://www.securecoding.cert.org/confluence/display/seccode/ENV34-C.+Do+not+store+pointers+returned+by+certain+functions Someone's apparently seen this before (but didn't report it?) - http://sourceforge.net/p/edk2/mailman/edk2-devel/thread/66BD57653246D24E9698B0A6509545A86DDB863C@ORSMSX109.amr.corp.intel.com/ |
|||
| msg229792 - (view) | Author: Aidan Hobson Sayers (aidanhs) | Date: 2014-10-21 21:17 | |
In case it matters, I'm compiling using Emscripten which implements getenv like so: https://github.com/kripken/emscripten/blob/1.25.2/src/library.js#L3323 (I personally think it's a bizarre way to do it, but technically I think it's ok?) |
|||
| msg321341 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2018-07-09 18:46 | |
New changeset f60bf0e168255b7675a4c049250ba6b202f8e647 by Serhiy Storchaka in branch 'master': bpo-22689: Copy the result of getenv() in sys_breakpointhook(). (GH-8194) https://github.com/python/cpython/commit/f60bf0e168255b7675a4c049250ba6b202f8e647 |
|||
| msg321342 - (view) | Author: miss-islington (miss-islington) | Date: 2018-07-09 19:06 | |
New changeset 6f4fbf8ea428e13959a7aaba0ac9725ed407752a by Miss Islington (bot) in branch '3.7': bpo-22689: Copy the result of getenv() in sys_breakpointhook(). (GH-8194) https://github.com/python/cpython/commit/6f4fbf8ea428e13959a7aaba0ac9725ed407752a |
|||
| msg321397 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2018-07-10 22:16 | |
> char *_rtpypath = Py_GETENV("PYTHONPATH"); /* XXX use wide version on Windows */
Python now copies the env var. In master, Modules/main.c:
int res = config_get_env_var_dup(&path, L"PYTHONPATH", "PYTHONPATH");
Moreover, bytes are decoded to Unicode (wchar_t) on UNIX.
This issue is now 4 years old and Serhiy just fixed one issue, so I close the issue.
Even if there is a risk of an issue, nobody came up with a concrete way to trigger a bug, so I don't think that it's a big issue. For example, the reported bug was on Py_GETENV("PYTHONPATH"), whereas this code is critical for Python: if it fails, everybody will complain. Except that since the bug has been reported, nobody ever saw an issue with this code. The code is part of the early code to initialize Python, when there is not possible to execute arbitrary code nor have a second thread, so we should be fine.
|
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:09 | admin | set | github: 66879 |
| 2018-07-10 22:16:35 | vstinner | set | status: open -> closed versions: + Python 3.8, - Python 2.7, Python 3.4, Python 3.5 messages: + msg321397 resolution: fixed |
| 2018-07-09 19:06:06 | miss-islington | set | nosy:
+ miss-islington messages: + msg321342 |
| 2018-07-09 18:47:15 | miss-islington | set | pull_requests: + pull_request7755 |
| 2018-07-09 18:46:54 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages: + msg321341 |
| 2018-07-09 09:53:35 | serhiy.storchaka | set | keywords:
+ patch stage: patch review pull_requests: + pull_request7746 |
| 2014-10-21 21:17:38 | aidanhs | set | messages: + msg229792 |
| 2014-10-21 21:11:59 | vstinner | set | nosy:
+ vstinner versions: - Python 3.2, Python 3.3, Python 3.6 |
| 2014-10-21 21:03:44 | aidanhs | create | |
