Skip to content

Commit d48cc82

Browse files
y5c4l3y5c4l3
authored
pythongh-124651: Quote template strings in venv activation scripts (pythonGH-124712)
This patch properly quotes template strings in `venv` activation scripts. This mitigates potential command injection.
1 parent 44f841f commit d48cc82

File tree

7 files changed

+135
-21
lines changed

7 files changed

+135
-21
lines changed

‎Lib/test/test_venv.py‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import sys
1818
import sysconfig
1919
import tempfile
20+
import shlex
2021
from test.support import (captured_stdout, captured_stderr,
2122
skip_if_broken_multiprocessing_synchronize, verbose,
2223
requires_subprocess, is_android, is_apple_mobile,
@@ -110,6 +111,10 @@ def get_text_file_contents(self, *args, encoding='utf-8'):
110111
result = f.read()
111112
return result
112113

114+
def assertEndsWith(self, string, tail):
115+
if not string.endswith(tail):
116+
self.fail(f"String {string!r} does not end with {tail!r}")
117+
113118
class BasicTest(BaseTest):
114119
"""Test venv module functionality."""
115120

@@ -488,6 +493,82 @@ def test_executable_symlinks(self):
488493
'import sys; print(sys.executable)'])
489494
self.assertEqual(out.strip(), envpy.encode())
490495

496+
# gh-124651: test quoted strings
497+
@unittest.skipIf(os.name == 'nt', 'contains invalid characters on Windows')
498+
def test_special_chars_bash(self):
499+
"""
500+
Test that the template strings are quoted properly (bash)
501+
"""
502+
rmtree(self.env_dir)
503+
bash = shutil.which('bash')
504+
if bash is None:
505+
self.skipTest('bash required for this test')
506+
env_name = '"\';&&$e|\'"'
507+
env_dir = os.path.join(os.path.realpath(self.env_dir), env_name)
508+
builder = venv.EnvBuilder(clear=True)
509+
builder.create(env_dir)
510+
activate = os.path.join(env_dir, self.bindir, 'activate')
511+
test_script = os.path.join(self.env_dir, 'test_special_chars.sh')
512+
with open(test_script, "w") as f:
513+
f.write(f'source {shlex.quote(activate)}\n'
514+
'python -c \'import sys; print(sys.executable)\'\n'
515+
'python -c \'import os; print(os.environ["VIRTUAL_ENV"])\'\n'
516+
'deactivate\n')
517+
out, err = check_output([bash, test_script])
518+
lines = out.splitlines()
519+
self.assertTrue(env_name.encode() in lines[0])
520+
self.assertEndsWith(lines[1], env_name.encode())
521+
522+
# gh-124651: test quoted strings
523+
@unittest.skipIf(os.name == 'nt', 'contains invalid characters on Windows')
524+
def test_special_chars_csh(self):
525+
"""
526+
Test that the template strings are quoted properly (csh)
527+
"""
528+
rmtree(self.env_dir)
529+
csh = shutil.which('tcsh') or shutil.which('csh')
530+
if csh is None:
531+
self.skipTest('csh required for this test')
532+
env_name = '"\';&&$e|\'"'
533+
env_dir = os.path.join(os.path.realpath(self.env_dir), env_name)
534+
builder = venv.EnvBuilder(clear=True)
535+
builder.create(env_dir)
536+
activate = os.path.join(env_dir, self.bindir, 'activate.csh')
537+
test_script = os.path.join(self.env_dir, 'test_special_chars.csh')
538+
with open(test_script, "w") as f:
539+
f.write(f'source {shlex.quote(activate)}\n'
540+
'python -c \'import sys; print(sys.executable)\'\n'
541+
'python -c \'import os; print(os.environ["VIRTUAL_ENV"])\'\n'
542+
'deactivate\n')
543+
out, err = check_output([csh, test_script])
544+
lines = out.splitlines()
545+
self.assertTrue(env_name.encode() in lines[0])
546+
self.assertEndsWith(lines[1], env_name.encode())
547+
548+
# gh-124651: test quoted strings on Windows
549+
@unittest.skipUnless(os.name == 'nt', 'only relevant on Windows')
550+
def test_special_chars_windows(self):
551+
"""
552+
Test that the template strings are quoted properly on Windows
553+
"""
554+
rmtree(self.env_dir)
555+
env_name = "'&&^$e"
556+
env_dir = os.path.join(os.path.realpath(self.env_dir), env_name)
557+
builder = venv.EnvBuilder(clear=True)
558+
builder.create(env_dir)
559+
activate = os.path.join(env_dir, self.bindir, 'activate.bat')
560+
test_batch = os.path.join(self.env_dir, 'test_special_chars.bat')
561+
with open(test_batch, "w") as f:
562+
f.write('@echo off\n'
563+
f'"{activate}" & '
564+
f'{self.exe} -c "import sys; print(sys.executable)" & '
565+
f'{self.exe} -c "import os; print(os.environ[\'VIRTUAL_ENV\'])" & '
566+
'deactivate')
567+
out, err = check_output([test_batch])
568+
lines = out.splitlines()
569+
self.assertTrue(env_name.encode() in lines[0])
570+
self.assertEndsWith(lines[1], env_name.encode())
571+
491572
@unittest.skipUnless(os.name == 'nt', 'only relevant on Windows')
492573
def test_unicode_in_batch_file(self):
493574
"""

‎Lib/venv/__init__.py‎

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import sys
1212
import sysconfig
1313
import types
14+
import shlex
1415

1516

1617
CORE_VENV_DEPS = ('pip',)
@@ -484,11 +485,41 @@ def replace_variables(self, text, context):
484485
:param context: The information for the environment creation request
485486
being processed.
486487
"""
487-
text = text.replace('__VENV_DIR__', context.env_dir)
488-
text = text.replace('__VENV_NAME__', context.env_name)
489-
text = text.replace('__VENV_PROMPT__', context.prompt)
490-
text = text.replace('__VENV_BIN_NAME__', context.bin_name)
491-
text = text.replace('__VENV_PYTHON__', context.env_exe)
488+
replacements = {
489+
'__VENV_DIR__': context.env_dir,
490+
'__VENV_NAME__': context.env_name,
491+
'__VENV_PROMPT__': context.prompt,
492+
'__VENV_BIN_NAME__': context.bin_name,
493+
'__VENV_PYTHON__': context.env_exe,
494+
}
495+
496+
def quote_ps1(s):
497+
"""
498+
This should satisfy PowerShell quoting rules [1], unless the quoted
499+
string is passed directly to Windows native commands [2].
500+
[1]: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_quoting_rules
501+
[2]: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_parsing#passing-arguments-that-contain-quote-characters
502+
"""
503+
s = s.replace("'", "''")
504+
return f"'{s}'"
505+
506+
def quote_bat(s):
507+
return s
508+
509+
# gh-124651: need to quote the template strings properly
510+
quote = shlex.quote
511+
script_path = context.script_path
512+
if script_path.endswith('.ps1'):
513+
quote = quote_ps1
514+
elif script_path.endswith('.bat'):
515+
quote = quote_bat
516+
else:
517+
# fallbacks to POSIX shell compliant quote
518+
quote = shlex.quote
519+
520+
replacements = {key: quote(s) for key, s in replacements.items()}
521+
for key, quoted in replacements.items():
522+
text = text.replace(key, quoted)
492523
return text
493524

494525
def install_scripts(self, context, path):
@@ -538,6 +569,7 @@ def skip_file(f):
538569
with open(srcfile, 'rb') as f:
539570
data = f.read()
540571
try:
572+
context.script_path = srcfile
541573
new_data = (
542574
self.replace_variables(data.decode('utf-8'), context)
543575
.encode('utf-8')

‎Lib/venv/scripts/common/activate‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,20 +41,20 @@ case "$(uname)" in
4141
CYGWIN*|MSYS*|MINGW*)
4242
# transform D:\path\to\venv to /d/path/to/venv on MSYS and MINGW
4343
# and to /cygdrive/d/path/to/venv on Cygwin
44-
VIRTUAL_ENV=$(cygpath "__VENV_DIR__")
44+
VIRTUAL_ENV=$(cygpath __VENV_DIR__)
4545
export VIRTUAL_ENV
4646
;;
4747
*)
4848
# use the path as-is
49-
export VIRTUAL_ENV="__VENV_DIR__"
49+
export VIRTUAL_ENV=__VENV_DIR__
5050
;;
5151
esac
5252

5353
_OLD_VIRTUAL_PATH="$PATH"
54-
PATH="$VIRTUAL_ENV/__VENV_BIN_NAME__:$PATH"
54+
PATH="$VIRTUAL_ENV/"__VENV_BIN_NAME__":$PATH"
5555
export PATH
5656

57-
VIRTUAL_ENV_PROMPT="__VENV_PROMPT__"
57+
VIRTUAL_ENV_PROMPT=__VENV_PROMPT__
5858
export VIRTUAL_ENV_PROMPT
5959

6060
# unset PYTHONHOME if set
@@ -67,7 +67,7 @@ fi
6767

6868
if [ -z "${VIRTUAL_ENV_DISABLE_PROMPT:-}" ] ; then
6969
_OLD_VIRTUAL_PS1="${PS1:-}"
70-
PS1="(__VENV_PROMPT__) ${PS1:-}"
70+
PS1="("__VENV_PROMPT__") ${PS1:-}"
7171
export PS1
7272
fi
7373

‎Lib/venv/scripts/common/activate.fish‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,11 @@ end
3333
# Unset irrelevant variables.
3434
deactivate nondestructive
3535

36-
set -gx VIRTUAL_ENV "__VENV_DIR__"
36+
set -gx VIRTUAL_ENV __VENV_DIR__
3737

3838
set -gx _OLD_VIRTUAL_PATH $PATH
39-
set -gx PATH "$VIRTUAL_ENV/__VENV_BIN_NAME__" $PATH
40-
set -gx VIRTUAL_ENV_PROMPT "__VENV_PROMPT__"
39+
set -gx PATH "$VIRTUAL_ENV/"__VENV_BIN_NAME__ $PATH
40+
set -gx VIRTUAL_ENV_PROMPT __VENV_PROMPT__
4141

4242
# Unset PYTHONHOME if set.
4343
if set -q PYTHONHOME
@@ -57,7 +57,7 @@ if test -z "$VIRTUAL_ENV_DISABLE_PROMPT"
5757
set -l old_status $status
5858

5959
# Output the venv prompt; color taken from the blue of the Python logo.
60-
printf "%s(%s)%s " (set_color 4B8BBE) "__VENV_PROMPT__" (set_color normal)
60+
printf "%s(%s)%s " (set_color 4B8BBE) __VENV_PROMPT__ (set_color normal)
6161

6262
# Restore the return status of the previous command.
6363
echo "exit $old_status" | .

‎Lib/venv/scripts/nt/activate.bat‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ if defined _OLD_CODEPAGE (
88
"%SystemRoot%\System32\chcp.com" 65001 > nul
99
)
1010

11-
set VIRTUAL_ENV=__VENV_DIR__
11+
set "VIRTUAL_ENV=__VENV_DIR__"
1212

1313
if not defined PROMPT set PROMPT=$P$G
1414

@@ -24,8 +24,8 @@ set PYTHONHOME=
2424
if defined _OLD_VIRTUAL_PATH set PATH=%_OLD_VIRTUAL_PATH%
2525
if not defined _OLD_VIRTUAL_PATH set _OLD_VIRTUAL_PATH=%PATH%
2626

27-
set PATH=%VIRTUAL_ENV%\__VENV_BIN_NAME__;%PATH%
28-
set VIRTUAL_ENV_PROMPT=__VENV_PROMPT__
27+
set "PATH=%VIRTUAL_ENV%\__VENV_BIN_NAME__;%PATH%"
28+
set "VIRTUAL_ENV_PROMPT=__VENV_PROMPT__"
2929

3030
:END
3131
if defined _OLD_CODEPAGE (

‎Lib/venv/scripts/posix/activate.csh‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,17 @@ alias deactivate 'test $?_OLD_VIRTUAL_PATH != 0 && setenv PATH "$_OLD_VIRTUAL_PA
99
# Unset irrelevant variables.
1010
deactivate nondestructive
1111

12-
setenv VIRTUAL_ENV "__VENV_DIR__"
12+
setenv VIRTUAL_ENV __VENV_DIR__
1313

1414
set _OLD_VIRTUAL_PATH="$PATH"
15-
setenv PATH "$VIRTUAL_ENV/__VENV_BIN_NAME__:$PATH"
16-
setenv VIRTUAL_ENV_PROMPT "__VENV_PROMPT__"
15+
setenv PATH "$VIRTUAL_ENV/"__VENV_BIN_NAME__":$PATH"
16+
setenv VIRTUAL_ENV_PROMPT __VENV_PROMPT__
1717

1818

1919
set _OLD_VIRTUAL_PROMPT="$prompt"
2020

2121
if (! "$?VIRTUAL_ENV_DISABLE_PROMPT") then
22-
set prompt = "(__VENV_PROMPT__) $prompt:q"
22+
set prompt = "("__VENV_PROMPT__") $prompt:q"
2323
endif
2424

2525
alias pydoc python -m pydoc

‎Misc/NEWS.d/next/Library/2024-09-28-02-03-04.gh-issue-124651.bLBGtH.rst‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Properly quote template strings in :mod:`venv` activation scripts.

0 commit comments

Comments
 (0)