From 82dbaffa36fe317e331b2d22c5efbb6a0b6c7b19 Mon Sep 17 00:00:00 2001

From: Lumir Balhar <lbalhar@redhat.com>

Date: Tue, 8 Oct 2024 09:15:09 +0200

Subject: [PATCH] Prevent command injection

---

 docs/changelog/2768.bugfix.rst                |  2 ++

 src/virtualenv/activation/bash/activate.sh    |  9 +++++----

 src/virtualenv/activation/batch/__init__.py   |  4 ++++

 src/virtualenv/activation/cshell/activate.csh | 10 +++++-----

 src/virtualenv/activation/fish/activate.fish  |  8 ++++----

 src/virtualenv/activation/nushell/__init__.py | 19 ++++++++++++++++++

 src/virtualenv/activation/nushell/activate.nu |  8 ++++----

 .../activation/powershell/__init__.py         | 12 +++++++++++

 .../activation/powershell/activate.ps1        |  6 +++---

 src/virtualenv/activation/python/__init__.py  |  6 +++++-

 .../activation/python/activate_this.py        |  6 +++---

 src/virtualenv/activation/via_template.py     | 15 ++++++++++++--

 tests/conftest.py                             |  6 +++++-

 tests/unit/activation/conftest.py             |  3 +--

 tests/unit/activation/test_batch.py           | 10 +++++-----

 tests/unit/activation/test_powershell.py      | 20 ++++++++++++++-----

 16 files changed, 105 insertions(+), 39 deletions(-)

 create mode 100644 docs/changelog/2768.bugfix.rst

diff --git a/docs/changelog/2768.bugfix.rst b/docs/changelog/2768.bugfix.rst

new file mode 100644

index 0000000..7651eb6

--- /dev/null

+++ 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/src/virtualenv/activation/bash/activate.sh b/src/virtualenv/activation/bash/activate.sh

index fb40db6..d047ea4 100644

--- 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,8 +62,9 @@ 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

+        PROMPT=__VIRTUAL_PROMPT__

+        PS1="(${PROMPT}) ${PS1-}"

     else

         PS1="(`basename \"$VIRTUAL_ENV\"`) ${PS1-}"

     fi

diff --git a/src/virtualenv/activation/batch/__init__.py b/src/virtualenv/activation/batch/__init__.py

index 13ba097..b3e8540 100644

--- a/src/virtualenv/activation/batch/__init__.py

+++ b/src/virtualenv/activation/batch/__init__.py

@@ -13,6 +13,10 @@ class BatchActivator(ViaTemplateActivator):

         yield "deactivate.bat"

         yield "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().instantiate_template(replacements, template, creator)

diff --git a/src/virtualenv/activation/cshell/activate.csh b/src/virtualenv/activation/cshell/activate.csh

index 837dcda..fcec426 100644

--- 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

@@ -42,7 +42,7 @@ if ( $do_prompt == "1" ) then

         if ( "$prompt:q" =~ *"$newline:q"* ) then

             :

         else

-            set prompt = "$env_name:q$prompt:q"

+            set prompt = '('"$env_name:q"') '"$prompt:q"

         endif

     endif

 endif

diff --git a/src/virtualenv/activation/fish/activate.fish b/src/virtualenv/activation/fish/activate.fish

index 62f631e..3637d55 100644

--- a/src/virtualenv/activation/fish/activate.fish

+++ b/src/virtualenv/activation/fish/activate.fish

@@ -57,7 +57,7 @@ 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

@@ -65,7 +65,7 @@ if test (echo $FISH_VERSION | head -c 1) -lt 3

 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

index 4e2ea77..c96af22 100644

--- a/src/virtualenv/activation/nushell/__init__.py

+++ b/src/virtualenv/activation/nushell/__init__.py

@@ -5,6 +5,25 @@ class NushellActivator(ViaTemplateActivator):

     def templates(self):

         yield "activate.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):  # noqa: U100

         return {

             "__VIRTUAL_PROMPT__": "" if self.flag_prompt is None else self.flag_prompt,

diff --git a/src/virtualenv/activation/nushell/activate.nu b/src/virtualenv/activation/nushell/activate.nu

index 3da1519..569a8a1 100644

--- a/src/virtualenv/activation/nushell/activate.nu

+++ b/src/virtualenv/activation/nushell/activate.nu

@@ -31,8 +31,8 @@ export-env {

     }

     let is_windows = ($nu.os-info.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 = (char esep)

     let path_name = (if $is_windows {

         if (has-env 'Path') {

@@ -73,10 +73,10 @@ export-env {

       $new_env

     } else {

       # 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

index 4e74ecb..773d690 100644

--- a/src/virtualenv/activation/powershell/__init__.py

+++ b/src/virtualenv/activation/powershell/__init__.py

@@ -5,6 +5,18 @@ class PowerShellActivator(ViaTemplateActivator):

     def templates(self):

         yield "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}'"

+

 __all__ = [

     "PowerShellActivator",

diff --git a/src/virtualenv/activation/powershell/activate.ps1 b/src/virtualenv/activation/powershell/activate.ps1

index d524347..346980d 100644

--- a/src/virtualenv/activation/powershell/activate.ps1

+++ b/src/virtualenv/activation/powershell/activate.ps1

@@ -35,18 +35,18 @@ $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

-            ("(__VIRTUAL_PROMPT__) " + $previous_prompt_value)

+            ((__VIRTUAL_PROMPT__) + $previous_prompt_value)

         }

     }

     else {

diff --git a/src/virtualenv/activation/python/__init__.py b/src/virtualenv/activation/python/__init__.py

index a49444b..8f0971e 100644

--- a/src/virtualenv/activation/python/__init__.py

+++ b/src/virtualenv/activation/python/__init__.py

@@ -9,10 +9,14 @@ class PythonActivator(ViaTemplateActivator):

     def templates(self):

         yield "activate_this.py"

+    @staticmethod

+    def quote(string):

+        return repr(string)

+

     def replacements(self, creator, dest_folder):

         replacements = super().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()).replace("\\", "\\\\")  # escape Windows path characters

+        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

index e8eeb84..976952e 100644

--- a/src/virtualenv/activation/python/activate_this.py

+++ b/src/virtualenv/activation/python/activate_this.py

@@ -14,7 +14,7 @@ except NameError:

     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))

@@ -22,9 +22,9 @@ 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

index cc9dbda..b1dfa6b 100644

--- a/src/virtualenv/activation/via_template.py

+++ b/src/virtualenv/activation/via_template.py

@@ -1,4 +1,5 @@

 import os

+import shlex

 import sys

 from abc import ABCMeta, abstractmethod

@@ -19,6 +20,16 @@ class ViaTemplateActivator(Activator, metaclass=ABCMeta):

     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,8 +69,8 @@ class ViaTemplateActivator(Activator, metaclass=ABCMeta):

         binary = read_binary(self.__module__, template)

         text = binary.decode("utf-8", errors="strict")

         for key, value in replacements.items():

-            value = self._repr_unicode(creator, value)

-            text = text.replace(key, value)

+            value_uni = self._repr_unicode(creator, value)

+            text = text.replace(key, self.quote(value_uni))

         return text

     @staticmethod

diff --git a/tests/conftest.py b/tests/conftest.py

index a7ec4e0..b2fae66 100644

--- a/tests/conftest.py

+++ b/tests/conftest.py

@@ -292,7 +292,11 @@ 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

index e24a4fc..2d8b3ce 100644

--- a/tests/unit/activation/conftest.py

+++ b/tests/unit/activation/conftest.py

@@ -4,7 +4,6 @@ import subprocess

 import sys

 from os.path import dirname, normcase

 from pathlib import Path

-from shlex import quote

 from subprocess import Popen

 import pytest

@@ -146,7 +145,7 @@ class ActivationTester:

         assert out[-1] == "None", raw

     def quote(self, s):

-        return quote(s)

+        return self.of_class.quote(s)

     def python_cmd(self, cmd):

         return f"{os.path.basename(sys.executable)} -c {self.quote(cmd)}"

diff --git a/tests/unit/activation/test_batch.py b/tests/unit/activation/test_batch.py

index 9e6d6d6..e95f7ea 100644

--- a/tests/unit/activation/test_batch.py

+++ b/tests/unit/activation/test_batch.py

@@ -1,5 +1,3 @@

-from shlex import quote

-

 import pytest

 from virtualenv.activation import BatchActivator

@@ -25,10 +23,12 @@ def test_batch(activation_tester_class, activation_tester, tmp_path):

             return ["@echo off", "", "chcp 65001 1>NUL"] + super()._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

index 761237f..f495353 100644

--- a/tests/unit/activation/test_powershell.py

+++ b/tests/unit/activation/test_powershell.py

@@ -1,5 +1,4 @@

 import sys

-from shlex import quote

 import pytest

@@ -19,10 +18,6 @@ def test_powershell(activation_tester_class, activation_tester, monkeypatch):

             self.activate_cmd = "."

             self.script_encoding = "utf-16"

-        def quote(self, s):

-            """powershell 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()._get_test_lines(activate_script)

@@ -33,4 +28,19 @@ def test_powershell(activation_tester_class, activation_tester, monkeypatch):

         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)

-- 

2.46.2