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.

classification
Title: Posix getenv makes no guarantee of lifetime of returned string
Type: crash Stage: resolved
Components: Interpreter Core Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: aidanhs, miss-islington, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

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) * (Python committer) 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) * (Python committer) 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:09adminsetgithub: 66879
2018-07-10 22:16:35vstinnersetstatus: open -> closed
versions: + Python 3.8, - Python 2.7, Python 3.4, Python 3.5
messages: + msg321397

resolution: fixed
stage: patch review -> resolved
2018-07-09 19:06:06miss-islingtonsetnosy: + miss-islington
messages: + msg321342
2018-07-09 18:47:15miss-islingtonsetpull_requests: + pull_request7755
2018-07-09 18:46:54serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg321341
2018-07-09 09:53:35serhiy.storchakasetkeywords: + patch
stage: patch review
pull_requests: + pull_request7746
2014-10-21 21:17:38aidanhssetmessages: + msg229792
2014-10-21 21:11:59vstinnersetnosy: + vstinner

versions: - Python 3.2, Python 3.3, Python 3.6
2014-10-21 21:03:44aidanhscreate