From d48cc82ed25e26b02eb97c6263d95dcaa1e9111b Mon Sep 17 00:00:00 2001 From: Y5 <124019959+y5c4l3@users.noreply.github.com> Date: Tue, 22 Oct 2024 04:48:04 +0800 Subject: [PATCH] gh-124651: Quote template strings in `venv` activation scripts (GH-124712) This patch properly quotes template strings in `venv` activation scripts. This mitigates potential command injection. --- Lib/test/test_venv.py | 81 +++++++++++++++++++ Lib/venv/__init__.py | 42 ++++++++-- Lib/venv/scripts/common/activate | 10 +-- Lib/venv/scripts/common/activate.fish | 8 +- Lib/venv/scripts/nt/activate.bat | 6 +- Lib/venv/scripts/posix/activate.csh | 8 +- ...-09-28-02-03-04.gh-issue-124651.bLBGtH.rst | 1 + 7 files changed, 135 insertions(+), 21 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-09-28-02-03-04.gh-issue-124651.bLBGtH.rst diff --git a/Lib/test/test_venv.py b/Lib/test/test_venv.py index 1ef08da326c18c..6b2127bd31e40a 100644 --- a/Lib/test/test_venv.py +++ b/Lib/test/test_venv.py @@ -17,6 +17,7 @@ import sys import sysconfig import tempfile +import shlex from test.support import (captured_stdout, captured_stderr, skip_if_broken_multiprocessing_synchronize, verbose, requires_subprocess, is_android, is_apple_mobile, @@ -110,6 +111,10 @@ def get_text_file_contents(self, *args, encoding='utf-8'): result = f.read() return result + def assertEndsWith(self, string, tail): + if not string.endswith(tail): + self.fail(f"String {string!r} does not end with {tail!r}") + class BasicTest(BaseTest): """Test venv module functionality.""" @@ -488,6 +493,82 @@ def test_executable_symlinks(self): 'import sys; print(sys.executable)']) self.assertEqual(out.strip(), envpy.encode()) + # gh-124651: test quoted strings + @unittest.skipIf(os.name == 'nt', 'contains invalid characters on Windows') + def test_special_chars_bash(self): + """ + Test that the template strings are quoted properly (bash) + """ + rmtree(self.env_dir) + bash = shutil.which('bash') + if bash is None: + self.skipTest('bash required for this test') + env_name = '"\';&&$e|\'"' + env_dir = os.path.join(os.path.realpath(self.env_dir), env_name) + builder = venv.EnvBuilder(clear=True) + builder.create(env_dir) + activate = os.path.join(env_dir, self.bindir, 'activate') + test_script = os.path.join(self.env_dir, 'test_special_chars.sh') + with open(test_script, "w") as f: + f.write(f'source {shlex.quote(activate)}\n' + 'python -c \'import sys; print(sys.executable)\'\n' + 'python -c \'import os; print(os.environ["VIRTUAL_ENV"])\'\n' + 'deactivate\n') + out, err = check_output([bash, test_script]) + lines = out.splitlines() + self.assertTrue(env_name.encode() in lines[0]) + self.assertEndsWith(lines[1], env_name.encode()) + + # gh-124651: test quoted strings + @unittest.skipIf(os.name == 'nt', 'contains invalid characters on Windows') + def test_special_chars_csh(self): + """ + Test that the template strings are quoted properly (csh) + """ + rmtree(self.env_dir) + csh = shutil.which('tcsh') or shutil.which('csh') + if csh is None: + self.skipTest('csh required for this test') + env_name = '"\';&&$e|\'"' + env_dir = os.path.join(os.path.realpath(self.env_dir), env_name) + builder = venv.EnvBuilder(clear=True) + builder.create(env_dir) + activate = os.path.join(env_dir, self.bindir, 'activate.csh') + test_script = os.path.join(self.env_dir, 'test_special_chars.csh') + with open(test_script, "w") as f: + f.write(f'source {shlex.quote(activate)}\n' + 'python -c \'import sys; print(sys.executable)\'\n' + 'python -c \'import os; print(os.environ["VIRTUAL_ENV"])\'\n' + 'deactivate\n') + out, err = check_output([csh, test_script]) + lines = out.splitlines() + self.assertTrue(env_name.encode() in lines[0]) + self.assertEndsWith(lines[1], env_name.encode()) + + # gh-124651: test quoted strings on Windows + @unittest.skipUnless(os.name == 'nt', 'only relevant on Windows') + def test_special_chars_windows(self): + """ + Test that the template strings are quoted properly on Windows + """ + rmtree(self.env_dir) + env_name = "'&&^$e" + env_dir = os.path.join(os.path.realpath(self.env_dir), env_name) + builder = venv.EnvBuilder(clear=True) + builder.create(env_dir) + activate = os.path.join(env_dir, self.bindir, 'activate.bat') + test_batch = os.path.join(self.env_dir, 'test_special_chars.bat') + with open(test_batch, "w") as f: + f.write('@echo off\n' + f'"{activate}" & ' + f'{self.exe} -c "import sys; print(sys.executable)" & ' + f'{self.exe} -c "import os; print(os.environ[\'VIRTUAL_ENV\'])" & ' + 'deactivate') + out, err = check_output([test_batch]) + lines = out.splitlines() + self.assertTrue(env_name.encode() in lines[0]) + self.assertEndsWith(lines[1], env_name.encode()) + @unittest.skipUnless(os.name == 'nt', 'only relevant on Windows') def test_unicode_in_batch_file(self): """ diff --git a/Lib/venv/__init__.py b/Lib/venv/__init__.py index a5d348ba4cf121..ca1af84e6705fe 100644 --- a/Lib/venv/__init__.py +++ b/Lib/venv/__init__.py @@ -11,6 +11,7 @@ import sys import sysconfig import types +import shlex CORE_VENV_DEPS = ('pip',) @@ -484,11 +485,41 @@ def replace_variables(self, text, context): :param context: The information for the environment creation request being processed. """ - text = text.replace('__VENV_DIR__', context.env_dir) - text = text.replace('__VENV_NAME__', context.env_name) - text = text.replace('__VENV_PROMPT__', context.prompt) - text = text.replace('__VENV_BIN_NAME__', context.bin_name) - text = text.replace('__VENV_PYTHON__', context.env_exe) + replacements = { + '__VENV_DIR__': context.env_dir, + '__VENV_NAME__': context.env_name, + '__VENV_PROMPT__': context.prompt, + '__VENV_BIN_NAME__': context.bin_name, + '__VENV_PYTHON__': context.env_exe, + } + + def quote_ps1(s): + """ + This should satisfy PowerShell quoting rules [1], unless the quoted + string is passed directly to Windows native commands [2]. + [1]: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_quoting_rules + [2]: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_parsing#passing-arguments-that-contain-quote-characters + """ + s = s.replace("'", "''") + return f"'{s}'" + + def quote_bat(s): + return s + + # gh-124651: need to quote the template strings properly + quote = shlex.quote + script_path = context.script_path + if script_path.endswith('.ps1'): + quote = quote_ps1 + elif script_path.endswith('.bat'): + quote = quote_bat + else: + # fallbacks to POSIX shell compliant quote + quote = shlex.quote + + replacements = {key: quote(s) for key, s in replacements.items()} + for key, quoted in replacements.items(): + text = text.replace(key, quoted) return text def install_scripts(self, context, path): @@ -538,6 +569,7 @@ def skip_file(f): with open(srcfile, 'rb') as f: data = f.read() try: + context.script_path = srcfile new_data = ( self.replace_variables(data.decode('utf-8'), context) .encode('utf-8') diff --git a/Lib/venv/scripts/common/activate b/Lib/venv/scripts/common/activate index 44f137672e9d2e..70673a265d41f8 100644 --- a/Lib/venv/scripts/common/activate +++ b/Lib/venv/scripts/common/activate @@ -41,20 +41,20 @@ case "$(uname)" in CYGWIN*|MSYS*|MINGW*) # transform D:\path\to\venv to /d/path/to/venv on MSYS and MINGW # and to /cygdrive/d/path/to/venv on Cygwin - VIRTUAL_ENV=$(cygpath "__VENV_DIR__") + VIRTUAL_ENV=$(cygpath __VENV_DIR__) export VIRTUAL_ENV ;; *) # use the path as-is - export VIRTUAL_ENV="__VENV_DIR__" + export VIRTUAL_ENV=__VENV_DIR__ ;; esac _OLD_VIRTUAL_PATH="$PATH" -PATH="$VIRTUAL_ENV/__VENV_BIN_NAME__:$PATH" +PATH="$VIRTUAL_ENV/"__VENV_BIN_NAME__":$PATH" export PATH -VIRTUAL_ENV_PROMPT="__VENV_PROMPT__" +VIRTUAL_ENV_PROMPT=__VENV_PROMPT__ export VIRTUAL_ENV_PROMPT # unset PYTHONHOME if set @@ -67,7 +67,7 @@ fi if [ -z "${VIRTUAL_ENV_DISABLE_PROMPT:-}" ] ; then _OLD_VIRTUAL_PS1="${PS1:-}" - PS1="(__VENV_PROMPT__) ${PS1:-}" + PS1="("__VENV_PROMPT__") ${PS1:-}" export PS1 fi diff --git a/Lib/venv/scripts/common/activate.fish b/Lib/venv/scripts/common/activate.fish index 25c42756789bbc..284a7469c99b57 100644 --- a/Lib/venv/scripts/common/activate.fish +++ b/Lib/venv/scripts/common/activate.fish @@ -33,11 +33,11 @@ end # Unset irrelevant variables. deactivate nondestructive -set -gx VIRTUAL_ENV "__VENV_DIR__" +set -gx VIRTUAL_ENV __VENV_DIR__ set -gx _OLD_VIRTUAL_PATH $PATH -set -gx PATH "$VIRTUAL_ENV/__VENV_BIN_NAME__" $PATH -set -gx VIRTUAL_ENV_PROMPT "__VENV_PROMPT__" +set -gx PATH "$VIRTUAL_ENV/"__VENV_BIN_NAME__ $PATH +set -gx VIRTUAL_ENV_PROMPT __VENV_PROMPT__ # Unset PYTHONHOME if set. if set -q PYTHONHOME @@ -57,7 +57,7 @@ if test -z "$VIRTUAL_ENV_DISABLE_PROMPT" set -l old_status $status # Output the venv prompt; color taken from the blue of the Python logo. - printf "%s(%s)%s " (set_color 4B8BBE) "__VENV_PROMPT__" (set_color normal) + printf "%s(%s)%s " (set_color 4B8BBE) __VENV_PROMPT__ (set_color normal) # Restore the return status of the previous command. echo "exit $old_status" | . diff --git a/Lib/venv/scripts/nt/activate.bat b/Lib/venv/scripts/nt/activate.bat index dd5ea8eb67b90a..35533e4b551155 100644 --- a/Lib/venv/scripts/nt/activate.bat +++ b/Lib/venv/scripts/nt/activate.bat @@ -8,7 +8,7 @@ if defined _OLD_CODEPAGE ( "%SystemRoot%\System32\chcp.com" 65001 > nul ) -set VIRTUAL_ENV=__VENV_DIR__ +set "VIRTUAL_ENV=__VENV_DIR__" if not defined PROMPT set PROMPT=$P$G @@ -24,8 +24,8 @@ set PYTHONHOME= if defined _OLD_VIRTUAL_PATH set PATH=%_OLD_VIRTUAL_PATH% if not defined _OLD_VIRTUAL_PATH set _OLD_VIRTUAL_PATH=%PATH% -set PATH=%VIRTUAL_ENV%\__VENV_BIN_NAME__;%PATH% -set VIRTUAL_ENV_PROMPT=__VENV_PROMPT__ +set "PATH=%VIRTUAL_ENV%\__VENV_BIN_NAME__;%PATH%" +set "VIRTUAL_ENV_PROMPT=__VENV_PROMPT__" :END if defined _OLD_CODEPAGE ( diff --git a/Lib/venv/scripts/posix/activate.csh b/Lib/venv/scripts/posix/activate.csh index b5db4a0f847e06..2a3fa835476ab9 100644 --- a/Lib/venv/scripts/posix/activate.csh +++ b/Lib/venv/scripts/posix/activate.csh @@ -9,17 +9,17 @@ alias deactivate 'test $?_OLD_VIRTUAL_PATH != 0 && setenv PATH "$_OLD_VIRTUAL_PA # Unset irrelevant variables. deactivate nondestructive -setenv VIRTUAL_ENV "__VENV_DIR__" +setenv VIRTUAL_ENV __VENV_DIR__ set _OLD_VIRTUAL_PATH="$PATH" -setenv PATH "$VIRTUAL_ENV/__VENV_BIN_NAME__:$PATH" -setenv VIRTUAL_ENV_PROMPT "__VENV_PROMPT__" +setenv PATH "$VIRTUAL_ENV/"__VENV_BIN_NAME__":$PATH" +setenv VIRTUAL_ENV_PROMPT __VENV_PROMPT__ set _OLD_VIRTUAL_PROMPT="$prompt" if (! "$?VIRTUAL_ENV_DISABLE_PROMPT") then - set prompt = "(__VENV_PROMPT__) $prompt:q" + set prompt = "("__VENV_PROMPT__") $prompt:q" endif alias pydoc python -m pydoc diff --git a/Misc/NEWS.d/next/Library/2024-09-28-02-03-04.gh-issue-124651.bLBGtH.rst b/Misc/NEWS.d/next/Library/2024-09-28-02-03-04.gh-issue-124651.bLBGtH.rst new file mode 100644 index 00000000000000..17fc9171390dd9 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-09-28-02-03-04.gh-issue-124651.bLBGtH.rst @@ -0,0 +1 @@ +Properly quote template strings in :mod:`venv` activation scripts.