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.

(cherry picked from commit d48cc82)

Author: y5c4l3

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_emscripten, is_wasi,
@@ -97,6 +98,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."""
 
@@ -446,6 +451,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):
         """

Lib/venv/__init__.py

@@ -11,6 +11,7 @@
 import sys
 import sysconfig
 import types
+import shlex
 
 
 CORE_VENV_DEPS = ('pip',)
@@ -422,11 +423,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):
@@ -465,16 +496,19 @@ def install_scripts(self, context, path):
                 dstfile = os.path.join(dstdir, f)
                 with open(srcfile, 'rb') as f:
                     data = f.read()
-                if not srcfile.endswith(('.exe', '.pdb')):
-                    try:
-                        data = data.decode('utf-8')
-                        data = self.replace_variables(data, context)
-                        data = data.encode('utf-8')
-                    except UnicodeError as e:
-                        data = None
-                        logger.warning('unable to copy script %r, '
-                                       'may be binary: %s', srcfile, e)
-                if data is not None:
+                try:
+                    context.script_path = srcfile
+                    new_data = (
+                        self.replace_variables(data.decode('utf-8'), context)
+                            .encode('utf-8')
+                    )
+                except UnicodeError as e:
+                    logger.warning('unable to copy script %r, '
+                                   'may be binary: %s', srcfile, e)
+                    continue
+                if new_data == data:
+                    shutil.copy2(srcfile, dstfile)
+                else:
                     with open(dstfile, 'wb') as f:
                         f.write(data)
                     shutil.copymode(srcfile, dstfile)

Lib/venv/scripts/common/activate

@@ -37,19 +37,26 @@ deactivate () {
 deactivate nondestructive
 
 # on Windows, a path can contain colons and backslashes and has to be converted:
-if [ "${OSTYPE:-}" = "cygwin" ] || [ "${OSTYPE:-}" = "msys" ] ; then
-    # transform D:\path\to\venv to /d/path/to/venv on MSYS
-    # and to /cygdrive/d/path/to/venv on Cygwin
-    export VIRTUAL_ENV=$(cygpath "__VENV_DIR__")
-else
-    # use the path as-is
-    export VIRTUAL_ENV="__VENV_DIR__"
-fi
+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__)
+        export VIRTUAL_ENV
+        ;;
+    *)
+        # use the path as-is
+        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__
+export VIRTUAL_ENV_PROMPT
+
 # unset PYTHONHOME if set
 # this will fail if PYTHONHOME is set to the empty string (which is bad anyway)
 # could use `if (set -u; : $PYTHONHOME) ;` in bash
@@ -60,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
     VIRTUAL_ENV_PROMPT="__VENV_PROMPT__"
     export VIRTUAL_ENV_PROMPT

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 (

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 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"
-    setenv VIRTUAL_ENV_PROMPT "__VENV_PROMPT__"
+    set prompt = "("__VENV_PROMPT__") $prompt:q"
 endif
 
 alias pydoc python -m pydoc

Lib/venv/scripts/posix/activate.fish

@@ -33,10 +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 PATH "$VIRTUAL_ENV/"__VENV_BIN_NAME__ $PATH
+set -gx VIRTUAL_ENV_PROMPT __VENV_PROMPT__
 
 # Unset PYTHONHOME if set.
 if set -q PYTHONHOME
@@ -56,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" | .

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.