Merge pull request #1 from ActiveState/BE-5124-2024-53899

icanhasmath · web-flow · commit f94742d84b45 · 2025-01-29T15:12:56.000-08:00
First pass at hand cherry-picking 86ddded…
diff --git a/docs/changelog/2768.bugfix.rst b/docs/changelog/2768.bugfix.rst
@@ -0,0 +1,2 @@
+Properly quote string placeholders in activation script templates to mitigate
+potential command injection - by :user:`y5c4l3`.
diff --git a/pyproject.toml b/pyproject.toml
@@ -17,3 +17,8 @@ directory = "docs/changelog"
 title_format = false
 issue_format = "`#{issue} <https://github.com/pypa/virtualenv/issues/{issue}>`_"
 template = "docs/changelog/template.jinja2"
+
+[tool.ruff]
+lint.per-file-ignores."src/virtualenv/activation/python/activate_this.py" = [
+  "F821", # ignore undefined template string placeholders
+]
diff --git a/src/virtualenv/activation/bash/activate.sh b/src/virtualenv/activation/bash/activate.sh
@@ -44,14 +44,14 @@ deactivate () {
 # unset irrelevant variables
 deactivate nondestructive
 
-VIRTUAL_ENV='__VIRTUAL_ENV__'
+VIRTUAL_ENV=__VIRTUAL_ENV__
 if ([ "$OSTYPE" = "cygwin" ] || [ "$OSTYPE" = "msys" ]) && $(command -v cygpath &> /dev/null) ; then
     VIRTUAL_ENV=$(cygpath -u "$VIRTUAL_ENV")
 fi
 export VIRTUAL_ENV
 
 _OLD_VIRTUAL_PATH="$PATH"
-PATH="$VIRTUAL_ENV/__BIN_NAME__:$PATH"
+PATH="$VIRTUAL_ENV/"__BIN_NAME__":$PATH"
 export PATH
 
 # unset PYTHONHOME if set
@@ -62,10 +62,10 @@ fi
 
 if [ -z "${VIRTUAL_ENV_DISABLE_PROMPT-}" ] ; then
     _OLD_VIRTUAL_PS1="${PS1-}"
-    if [ "x__VIRTUAL_PROMPT__" != x ] ; then
-        PS1="(__VIRTUAL_PROMPT__) ${PS1-}"
+    if [ "x"__VIRTUAL_PROMPT__ != x ] ; then
+        PS1=(__VIRTUAL_PROMPT__) ${PS1-}
     else
-        PS1="(`basename \"$VIRTUAL_ENV\"`) ${PS1-}"
+        PS1=(`basename \"$VIRTUAL_ENV\"`) ${PS1-}
     fi
     export PS1
 fi
diff --git a/src/virtualenv/activation/batch/__init__.py b/src/virtualenv/activation/batch/__init__.py
@@ -17,6 +17,10 @@ def templates(self):
         yield Path("deactivate.bat")
         yield Path("pydoc.bat")
 
+    @staticmethod
+    def quote(string):
+        return string
+
     def instantiate_template(self, replacements, template, creator):
         # ensure the text has all newlines as \r\n - required by batch
         base = super(BatchActivator, self).instantiate_template(replacements, template, creator)
diff --git a/src/virtualenv/activation/cshell/activate.csh b/src/virtualenv/activation/cshell/activate.csh
@@ -10,15 +10,15 @@ alias deactivate 'test $?_OLD_VIRTUAL_PATH != 0 && setenv PATH "$_OLD_VIRTUAL_PA
 # Unset irrelevant variables.
 deactivate nondestructive
 
-setenv VIRTUAL_ENV '__VIRTUAL_ENV__'
+setenv VIRTUAL_ENV __VIRTUAL_ENV__
 
 set _OLD_VIRTUAL_PATH="$PATH:q"
-setenv PATH "$VIRTUAL_ENV:q/__BIN_NAME__:$PATH:q"
+setenv PATH "$VIRTUAL_ENV:q/"__BIN_NAME__":$PATH:q"
 
 
 
-if ('__VIRTUAL_PROMPT__' != "") then
-    set env_name = '(__VIRTUAL_PROMPT__) '
+if (__VIRTUAL_PROMPT__ != "") then
+    set env_name = (__VIRTUAL_PROMPT__) 
 else
     set env_name = '('"$VIRTUAL_ENV:t:q"') '
 endif
diff --git a/src/virtualenv/activation/fish/activate.fish b/src/virtualenv/activation/fish/activate.fish
@@ -57,15 +57,15 @@ end
 # Unset irrelevant variables.
 deactivate nondestructive
 
-set -gx VIRTUAL_ENV '__VIRTUAL_ENV__'
+set -gx VIRTUAL_ENV __VIRTUAL_ENV__
 
 # https://github.com/fish-shell/fish-shell/issues/436 altered PATH handling
 if test (echo $FISH_VERSION | head -c 1) -lt 3
    set -gx _OLD_VIRTUAL_PATH (_bashify_path $PATH)
 else
     set -gx _OLD_VIRTUAL_PATH $PATH
 end
-set -gx PATH "$VIRTUAL_ENV"'/__BIN_NAME__' $PATH
+set -gx PATH "$VIRTUAL_ENV"'/'__BIN_NAME__ $PATH
 
 # Unset `$PYTHONHOME` if set.
 if set -q PYTHONHOME
@@ -87,8 +87,8 @@ if test -z "$VIRTUAL_ENV_DISABLE_PROMPT"
 
         # Prompt override provided?
         # If not, just prepend the environment name.
-        if test -n '__VIRTUAL_PROMPT__'
-            printf '(%s) ' '__VIRTUAL_PROMPT__'
+        if test -n __VIRTUAL_PROMPT__
+            printf '(%s) ' __VIRTUAL_PROMPT__
         else
             printf '(%s) ' (basename "$VIRTUAL_ENV")
         end
diff --git a/src/virtualenv/activation/nushell/__init__.py b/src/virtualenv/activation/nushell/__init__.py
@@ -13,6 +13,24 @@ def templates(self):
         yield Path("activate.nu")
         yield Path("deactivate.nu")
 
+    @staticmethod
+    def quote(string):
+        """
+        Nushell supports raw strings like: r###'this is a string'###.
+        This method finds the maximum continuous sharps in the string and then
+        quote it with an extra sharp.
+        """
+        max_sharps = 0
+        current_sharps = 0
+        for char in string:
+            if char == "#":
+                current_sharps += 1
+                max_sharps = max(current_sharps, max_sharps)
+            else:
+                current_sharps = 0
+        wrapping = "#" * (max_sharps + 1)
+        return f"r{wrapping}'{string}'{wrapping}"
+
     def replacements(self, creator, dest_folder):
         # Due to nushell scoping, it isn't easy to create a function that will
         # deactivate the environment. For that reason a __DEACTIVATE_PATH__
diff --git a/src/virtualenv/activation/nushell/activate.nu b/src/virtualenv/activation/nushell/activate.nu
@@ -9,8 +9,8 @@ def-env activate-virtualenv [] {
     }
 
     let is-windows = ((sys).host.name | str downcase) == 'windows'
-    let virtual-env = '__VIRTUAL_ENV__'
-    let bin = '__BIN_NAME__'
+    let virtual-env = __VIRTUAL_ENV__
+    let bin = __BIN_NAME__
     let path-sep = '__PATH_SEP__'
     let path-name = if $is-windows {
         if (has-env 'Path') {
@@ -43,10 +43,10 @@ def-env activate-virtualenv [] {
     let new-path = ($old-path | prepend $venv-path | str collect $path-sep)
 
     # Creating the new prompt for the session
-    let virtual-prompt = if ('__VIRTUAL_PROMPT__' == '') {
+    let virtual-prompt = if (__VIRTUAL_PROMPT__ == '') {
         $'(char lparen)($virtual-env | path basename)(char rparen) '
     } else {
-        '(__VIRTUAL_PROMPT__) '
+        (__VIRTUAL_PROMPT__)
     }
 
     # Back up the old prompt builder
diff --git a/src/virtualenv/activation/powershell/__init__.py b/src/virtualenv/activation/powershell/__init__.py
@@ -8,3 +8,14 @@
 class PowerShellActivator(ViaTemplateActivator):
     def templates(self):
         yield Path("activate.ps1")
+
+    @staticmethod
+    def quote(string):
+        """
+        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
+        """  # noqa: D205
+        string = string.replace("'", "''")
+        return f"'{string}'"
diff --git a/src/virtualenv/activation/powershell/activate.ps1 b/src/virtualenv/activation/powershell/activate.ps1
@@ -35,14 +35,14 @@ $env:VIRTUAL_ENV = $VIRTUAL_ENV
 
 New-Variable -Scope global -Name _OLD_VIRTUAL_PATH -Value $env:PATH
 
-$env:PATH = "$env:VIRTUAL_ENV/__BIN_NAME____PATH_SEP__" + $env:PATH
+$env:PATH = "$env:VIRTUAL_ENV/" + __BIN_NAME____PATH_SEP__ + $env:PATH
 if (!$env:VIRTUAL_ENV_DISABLE_PROMPT) {
     function global:_old_virtual_prompt {
         ""
     }
     $function:_old_virtual_prompt = $function:prompt
 
-    if ("__VIRTUAL_PROMPT__" -ne "") {
+    if (__VIRTUAL_PROMPT__ -ne "") {
         function global:prompt {
             # Add the custom prefix to the existing prompt
             $previous_prompt_value = & $function:_old_virtual_prompt
diff --git a/src/virtualenv/activation/python/__init__.py b/src/virtualenv/activation/python/__init__.py
@@ -14,9 +14,14 @@ class PythonActivator(ViaTemplateActivator):
     def templates(self):
         yield Path("activate_this.py")
 
+    @staticmethod
+    def quote(string):
+        return repr(string)
+
     def replacements(self, creator, dest_folder):
         replacements = super(PythonActivator, self).replacements(creator, dest_folder)
         lib_folders = OrderedDict((os.path.relpath(str(i), str(dest_folder)), None) for i in creator.libs)
+        lib_folders = os.pathsep.join(lib_folders.keys())
         win_py2 = creator.interpreter.platform == "win32" and creator.interpreter.version_info.major == 2
         replacements.update(
             {
diff --git a/src/virtualenv/activation/python/activate_this.py b/src/virtualenv/activation/python/activate_this.py
@@ -15,17 +15,17 @@
     raise AssertionError("You must use exec(open(this_file).read(), {'__file__': this_file}))")
 
 bin_dir = os.path.dirname(abs_file)
-base = bin_dir[: -len("__BIN_NAME__") - 1]  # strip away the bin part from the __file__, plus the path separator
+base = bin_dir[: -len(__BIN_NAME__) - 1]  # strip away the bin part from the __file__, plus the path separator
 
 # prepend bin to PATH (this file is inside the bin directory)
 os.environ["PATH"] = os.pathsep.join([bin_dir] + os.environ.get("PATH", "").split(os.pathsep))
 os.environ["VIRTUAL_ENV"] = base  # virtual env is right above bin directory
 
 # add the virtual environments libraries to the host python import mechanism
 prev_length = len(sys.path)
-for lib in "__LIB_FOLDERS__".split(os.pathsep):
+for lib in __LIB_FOLDERS__.split(os.pathsep):
     path = os.path.realpath(os.path.join(bin_dir, lib))
-    site.addsitedir(path.decode("utf-8") if "__DECODE_PATH__" else path)
+    site.addsitedir(path.decode("utf-8") if __DECODE_PATH__ else path)
 sys.path[:] = sys.path[prev_length:] + sys.path[0:prev_length]
 
 sys.real_prefix = sys.prefix
diff --git a/src/virtualenv/activation/via_template.py b/src/virtualenv/activation/via_template.py
@@ -1,6 +1,7 @@
 from __future__ import absolute_import, unicode_literals
 
 import os
+import shlex
 import sys
 from abc import ABCMeta, abstractmethod
 
@@ -22,6 +23,15 @@ class ViaTemplateActivator(Activator):
     def templates(self):
         raise NotImplementedError
 
+    @staticmethod
+    def quote(string):
+        """
+        Quote strings in the activation script.
+        :param string: the string to quote
+        :return: quoted string that works in the activation script
+        """
+        return shlex.quote(string)
+
     def generate(self, creator):
         dest_folder = creator.bin_dir
         replacements = self.replacements(creator, dest_folder)
@@ -58,7 +68,7 @@ def instantiate_template(self, replacements, template, creator):
         text = binary.decode("utf-8", errors="strict")
         for key, value in replacements.items():
             value = self._repr_unicode(creator, value)
-            text = text.replace(key, value)
+            text = text.replace(key, self.quote(value))
         return text
 
     @staticmethod
diff --git a/tests/conftest.py b/tests/conftest.py
@@ -299,7 +299,12 @@ def is_inside_ci():
 
 @pytest.fixture(scope="session")
 def special_char_name():
-    base = "e-$ èрт🚒♞中片-j"
+    base = "'\";&&e-$ èрт🚒♞中片-j"
+    if IS_WIN:
+        # get rid of invalid characters on Windows
+        base = base.replace('"', "")
+        base = base.replace(";", "")
+
     # workaround for pypy3 https://bitbucket.org/pypy/pypy/issues/3147/venv-non-ascii-support-windows
     encoding = "ascii" if IS_WIN else sys.getfilesystemencoding()
     # let's not include characters that the file system cannot encode)
diff --git a/tests/unit/activation/conftest.py b/tests/unit/activation/conftest.py
@@ -10,17 +10,12 @@
 import pytest
 import six
 
-from virtualenv.info import IS_PYPY, PY2, WIN_CPYTHON_2
+from virtualenv.info import IS_PYPY, WIN_CPYTHON_2
 from virtualenv.run import cli_run
 from virtualenv.util.path import Path
 from virtualenv.util.six import ensure_str, ensure_text
 from virtualenv.util.subprocess import Popen
 
-if PY2:
-    from pipes import quote
-else:
-    from shlex import quote
-
 
 class ActivationTester(object):
     def __init__(self, of_class, session, cmd, activate_script, extension):
@@ -161,7 +156,7 @@ def assert_output(self, out, raw, tmp_path):
         assert out[-1] == "None", raw
 
     def quote(self, s):
-        return quote(s)
+        return self.of_class.quote(s)
 
     def python_cmd(self, cmd):
         return "{} -c {}".format(os.path.basename(sys.executable), self.quote(cmd))
diff --git a/tests/unit/activation/test_batch.py b/tests/unit/activation/test_batch.py
@@ -1,12 +1,6 @@
 from __future__ import absolute_import, unicode_literals
 
 from virtualenv.activation import BatchActivator
-from virtualenv.info import PY2
-
-if PY2:
-    from pipes import quote
-else:
-    from shlex import quote
 
 
 def test_batch(activation_tester_class, activation_tester, tmp_path, activation_python):
@@ -28,10 +22,12 @@ def _get_test_lines(self, activate_script):
             return ["@echo off", "", "chcp 65001 1>NUL"] + super(Batch, self)._get_test_lines(activate_script)
 
         def quote(self, s):
-            """double quotes needs to be single, and single need to be double"""
-            return "".join(("'" if c == '"' else ('"' if c == "'" else c)) for c in quote(s))
+            if '"' in s or " " in s:
+                text = s.replace('"', r"\"")
+                return f'"{text}"'
+            return s
 
         def print_prompt(self):
-            return "echo %PROMPT%"
+            return 'echo "%PROMPT%"'
 
     activation_tester(Batch)
diff --git a/tests/unit/activation/test_powershell.py b/tests/unit/activation/test_powershell.py
@@ -5,12 +5,6 @@
 import pytest
 
 from virtualenv.activation import PowerShellActivator
-from virtualenv.info import PY2
-
-if PY2:
-    from pipes import quote
-else:
-    from shlex import quote
 
 
 @pytest.mark.slow
@@ -26,10 +20,6 @@ def __init__(self, session):
             self.activate_cmd = "."
             self.script_encoding = "utf-16"
 
-        def quote(self, s):
-            """powershell double double quote needed for quotes within single quotes"""
-            return quote(s).replace('"', '""')
-
         def _get_test_lines(self, activate_script):
             # for BATCH utf-8 support need change the character code page to 650001
             return super(PowerShell, self)._get_test_lines(activate_script)
@@ -40,4 +30,19 @@ def invoke_script(self):
         def print_prompt(self):
             return "prompt"
 
+        def quote(self, s):
+            """
+            Tester will pass strings to native commands on Windows so extra
+            parsing rules are used. Check `PowerShellActivator.quote` for more
+            details.
+            """
+            text = PowerShellActivator.quote(s)
+            return text.replace('"', '""') if sys.platform == "win32" else text
+
+        def activate_call(self, script):
+            # Commands are called without quotes in PowerShell
+            cmd = self.activate_cmd
+            scr = self.quote(str(script))
+            return f"{cmd} {scr}".strip()
+
     activation_tester(PowerShell)

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+Properly quote string placeholders in activation script templates to mitigate`
	`2`	+potential command injection - by :user:`y5c4l3`.