gh-143309: fix UAF in os.execve when the environment is concurrently mutated

[picnixz]

• Issue: Use-after-free in parse_envlist via re-entrant env.keys() or env.values() #143309

[picnixz]

Mmh. This is tricky for Windows. I don't have a Windows machine to know what happened there.

[picnixz]

@chris-eibl I know you're on Windows, so could you help me there please?

[chris-eibl]

Will have a closer look tomorrow

[chris-eibl]

@chris-eibl I know you're on Windows, so could you help me there please?

Ups, really tricky on Windows. I've found two different issues:

Edit: Created #143327

Details

The test case boils down to this smallest reproducer I can get:

import os, sys
import subprocess

code = """
import os, sys
args = [sys.executable, '-c', 'print(4711)']
os.execve(args[0], args, {})
"""

cmd_line = [sys.executable, '-X', 'faulthandler', '-c', code]
env_1 = os.environ.copy()
env_2 = {}
env_2['SYSTEMROOT'] = os.environ['SYSTEMROOT']
proc = subprocess.Popen(
    cmd_line, stdin=subprocess.PIPE,
    stdout=subprocess.PIPE, stderr=subprocess.PIPE,
    env=env_1)
with proc:
    try:
        out, err = proc.communicate()
    finally:
        proc.kill()
        subprocess._cleanup()
print("rc", proc.returncode)
print("out", out)
print("err", err)

Using env_1 crashes for me like in CI, interestingly the "smaller" env_2 works. It seems to be the combination of subprocess and os.execve. Without subprocess, I do not see this problem.

[picnixz]

Oh, so the problem is subprocess but not my fix?

[chris-eibl]

Secondly, the os.execv* family has problems with strings on Windows:

Edit: created #143328

Details

import os
import sys

args = [sys.executable, '-c', 'print("hello from execve")']
os.execve(args[0], args, {})

This results in

    print(hello from execve)
                ^^^^
SyntaxError: invalid syntax

And using "print('hello from execve')" results in

    print('hello
          ^
SyntaxError: unterminated string literal (detected at line 1)

Both work like expected in WSL. The only thing that somewhat works is omitting the spaces "print('hellofromexecve')" :-o

[chris-eibl]

I haven't found these two things reported as issues, yet, I think I should create two new issues?

With the above in mind, changing your test slightly to:

        args = [sys.executable, '-c', "print('hellofromexecve')"]
        os.execve(args[0], args, MyEnv())
        """

        env = {}
        env['__cleanenv'] = True  # signal to assert_python not to do a copy
                                  # of os.environ on its own

        rc, out, _ = assert_python_ok('-c', code, **env)
        self.assertEqual(rc, 0)
        self.assertIn(b"hellofromexecve", out)

let's it pass for me. Without your fix applied, it will fail with an access violation, due to the UAF 🚀

[picnixz]

I haven't found these two things reported as issues, yet, I think I should create two new issues?

Please do so and thank you for all this investigation!

[chris-eibl]

Oh, Win x64 now almost green in CI like for me. Unfortunately, arm64 and Win32 still crash. I will look at Win32, do not have an Arm machine ...

[chris-eibl]

Sorry, didn't want to also apply my suggestion - just suggest. Misclicked, hangover from yesterday ...

[picnixz]

You're hijacking my code! 😨

[picnixz]

Could the issue on Windows and general be caused by this:

        PyObject *keyval = PyUnicode_FromFormat("%U=%U", key2, val2);

[chris-eibl]

You're hijacking my code! 😨

And already apologized. Misclicked. Sorry again.

arm64 and Win32 still crash

Win32 is green for me. I've run

python -m test.test_os.test_os ExecTests.test_execve_env_concurrent_mutation_with_fspath

5 times sucessfully ...

[picnixz]

And already apologized. Misclicked. Sorry again.

Don't worry! I was overreacting but I wasn't offended :)

[chris-eibl]

5 times sucessfully ...

... and crashed after trying a 6th time. But I could not crash x64 after 20 invocations ...

PyObject *keyval = PyUnicode_FromFormat("%U=%U", key2, val2);

IDKN. Looks ok at a first glance?

[picnixz]

I actually wonder whether the issue is because of execve in general or the environment we build.

[picnixz]

Could it be actually because we're not forwarding env['SYSTEMROOT'] = os.environ['SYSTEMROOT']? For Windows, if we are lacking SYSTEMROOT, wouldn't it be an issue? maybe execve & co must also inherit SYSTEMROOT in this case?

[chris-eibl]

It get's autofilled here

cpython/Lib/test/support/script_helper.py

Lines 131 to 134 in 422ca07

	if sys.platform == 'win32':
	# Windows requires at least the SYSTEMROOT environment variable to
	# start Python.
	env['SYSTEMROOT'] = os.environ['SYSTEMROOT']

[picnixz]

Yes, but not when calling execve. The environment I'm passing to execve is an environment that gets mutated.

[picnixz]

Ok I give up, I have no idea why this crashes but I'm pretty sure the issue is after we call os.execve and not before. I also that changing the command to something that is not Python may work but I don't know which program to use that would always be available on Windows.

[chris-eibl]

after we call os.execve

Yes, definitely. Was always the case when I was debugging it, but cannot crash it anymore in x64 when the env is "lightweight" ...

[picnixz]

I think we should debug what the environment passed to execve (that is, after parse_envlist) is called. Would you be willing to check what the contents of the array is (with the patch) please? Also, x64 crashes with FT mode but not in regular build.

[picnixz]

Considering #137934, I think we will, for now, just skip the test on Windows. If even os.execve is buggy in plain C, then there is nothing we can do on our side.