Skip to content

Commit

Permalink
pythongh-124651: Quote template strings in venv activation scripts (p…
Browse files Browse the repository at this point in the history
…ythonGH-124712)

This patch properly quotes template strings in `venv` activation
scripts. This mitigates potential command injection.

(cherry picked from commit d48cc82)
  • Loading branch information
y5c4l3 authored and vstinner committed Oct 30, 2024
1 parent 679dfae commit 1408cc9
Show file tree
Hide file tree
Showing 7 changed files with 135 additions and 21 deletions.
81 changes: 81 additions & 0 deletions Lib/test/test_venv.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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."""

Expand Down Expand Up @@ -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):
"""
Expand Down
42 changes: 37 additions & 5 deletions Lib/venv/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import sys
import sysconfig
import types
import shlex


CORE_VENV_DEPS = ('pip',)
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -466,6 +497,7 @@ def install_scripts(self, context, path):
with open(srcfile, 'rb') as f:
data = f.read()
if not srcfile.endswith(('.exe', '.pdb')):
context.script_path = srcfile
try:
data = data.decode('utf-8')
data = self.replace_variables(data, context)
Expand Down
10 changes: 5 additions & 5 deletions Lib/venv/scripts/common/activate
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@ deactivate nondestructive
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__")
export VIRTUAL_ENV=$(cygpath __VENV_DIR__)
else
# use the path as-is
export VIRTUAL_ENV="__VENV_DIR__"
export VIRTUAL_ENV=__VENV_DIR__
fi

_OLD_VIRTUAL_PATH="$PATH"
PATH="$VIRTUAL_ENV/__VENV_BIN_NAME__:$PATH"
PATH="$VIRTUAL_ENV/"__VENV_BIN_NAME__":$PATH"
export PATH

# unset PYTHONHOME if set
Expand All @@ -60,9 +60,9 @@ 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__"
VIRTUAL_ENV_PROMPT=__VENV_PROMPT__
export VIRTUAL_ENV_PROMPT
fi

Expand Down
6 changes: 3 additions & 3 deletions Lib/venv/scripts/nt/activate.bat
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 (
Expand Down
8 changes: 4 additions & 4 deletions Lib/venv/scripts/posix/activate.csh
Original file line number Diff line number Diff line change
Expand Up @@ -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"


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"
setenv VIRTUAL_ENV_PROMPT __VENV_PROMPT__
endif

alias pydoc python -m pydoc
Expand Down
8 changes: 4 additions & 4 deletions Lib/venv/scripts/posix/activate.fish
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ 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

# Unset PYTHONHOME if set.
if set -q PYTHONHOME
Expand All @@ -56,7 +56,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" | .
Expand All @@ -65,5 +65,5 @@ if test -z "$VIRTUAL_ENV_DISABLE_PROMPT"
end

set -gx _OLD_FISH_PROMPT_OVERRIDE "$VIRTUAL_ENV"
set -gx VIRTUAL_ENV_PROMPT "__VENV_PROMPT__"
set -gx VIRTUAL_ENV_PROMPT __VENV_PROMPT__
end
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Properly quote template strings in :mod:`venv` activation scripts.

0 comments on commit 1408cc9

Please sign in to comment.