Skip to content

Commit 4d5a105

Browse files
gaborbernatgaborbernat
authored
🐛 fix(activation): stop path command injection in bash and fish (#3252)
1 parent 0525dce commit 4d5a105

5 files changed

Lines changed: 114 additions & 56 deletions

File tree

‎docs/changelog/3252.bugfix.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix ``activate`` and ``activate.fish`` running commands embedded in the virtual environment path or in the interpreter's
2+
Tcl/Tk library paths.

‎src/virtualenv/activation/bash/activate.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ deactivate () {
7272
deactivate nondestructive
7373

7474
if [ ! -d __VIRTUAL_ENV__ ]; then
75-
echo "Virtual environment directory __VIRTUAL_ENV__ does not exist!" >&2
75+
echo "Virtual environment directory" __VIRTUAL_ENV__ "does not exist!" >&2
7676
CURRENT_PATH=$(realpath "${BASH_SOURCE[0]}")
7777
CURRENT_DIR=$(dirname "${CURRENT_PATH}")
7878
VIRTUAL_ENV="$(realpath "${CURRENT_DIR}/../")"

‎src/virtualenv/activation/fish/activate.fish‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,11 @@ set -gx PATH "$VIRTUAL_ENV"'/'__BIN_NAME__ $PATH
8282
# an empty saved value tells deactivate to erase the variable
8383
if test -n __TCL_LIBRARY__
8484
set -gx _OLD_VIRTUAL_TCL_LIBRARY "$TCL_LIBRARY"
85-
set -gx TCL_LIBRARY '__TCL_LIBRARY__'
85+
set -gx TCL_LIBRARY __TCL_LIBRARY__
8686
end
8787
if test -n __TK_LIBRARY__
8888
set -gx _OLD_VIRTUAL_TK_LIBRARY "$TK_LIBRARY"
89-
set -gx TK_LIBRARY '__TK_LIBRARY__'
89+
set -gx TK_LIBRARY __TK_LIBRARY__
9090
end
9191

9292
# Prompt override provided?

‎tests/unit/activation/test_bash.py‎

Lines changed: 61 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,18 @@
55
import subprocess
66
import sys
77
from argparse import Namespace
8+
from typing import TYPE_CHECKING
89

910
import pytest
1011

1112
from virtualenv.activation import BashActivator
1213
from virtualenv.info import IS_WIN
1314
from virtualenv.run import cli_run
1415

16+
if TYPE_CHECKING:
17+
from collections.abc import Callable
18+
from pathlib import Path
19+
1520

1621
@pytest.mark.skipif(IS_WIN, reason="Github Actions ships with WSL bash")
1722
@pytest.mark.parametrize(
@@ -75,35 +80,68 @@ def __init__(self, dest) -> None:
7580
assert "export TCL_LIBRARY" in content
7681

7782

83+
@pytest.fixture
84+
def relocated_bash_venv(
85+
tmp_path: Path, current_fastest: str
86+
) -> Callable[[str], tuple[subprocess.CompletedProcess[str], Path, Path]]:
87+
def source_after_move(name: str) -> tuple[subprocess.CompletedProcess[str], Path, Path]:
88+
original = tmp_path / name
89+
cli_run([
90+
"--without-pip",
91+
str(original),
92+
"--creator",
93+
current_fastest,
94+
"--no-periodic-update",
95+
"--activators",
96+
"bash",
97+
])
98+
relocated = tmp_path / "relocated"
99+
shutil.move(original, relocated)
100+
work_dir = tmp_path / "workdir"
101+
work_dir.mkdir()
102+
result = subprocess.run(
103+
["bash", "-c", f'source "{relocated / "bin" / "activate"}" 2>/dev/null && echo "$VIRTUAL_ENV"'],
104+
capture_output=True,
105+
text=True,
106+
cwd=str(work_dir),
107+
encoding="utf-8",
108+
check=False,
109+
)
110+
return result, relocated, work_dir
111+
112+
return source_after_move
113+
114+
78115
@pytest.mark.skipif(IS_WIN, reason="Github Actions ships with WSL bash")
79-
def test_bash_activate_relocation_resolves_virtual_env(tmp_path, current_fastest) -> None:
80-
original = tmp_path / "original"
81-
cli_run([
82-
"--without-pip",
83-
str(original),
84-
"--creator",
85-
current_fastest,
86-
"--no-periodic-update",
87-
"--activators",
88-
"bash",
89-
])
90-
relocated = tmp_path / "relocated"
91-
shutil.move(original, relocated)
116+
@pytest.mark.parametrize(
117+
"name", [pytest.param("original", id="plain"), pytest.param("has(paren)and'quote", id="shell-metacharacters")]
118+
)
119+
def test_bash_activate_relocation_resolves_virtual_env(
120+
relocated_bash_venv: Callable[[str], tuple[subprocess.CompletedProcess[str], Path, Path]], name: str
121+
) -> None:
122+
result, relocated, _ = relocated_bash_venv(name)
92123

93-
work_dir = tmp_path / "workdir"
94-
work_dir.mkdir()
95-
activate_script = relocated / "bin" / "activate"
96-
result = subprocess.run(
97-
["bash", "-c", f'source "{activate_script}" 2>/dev/null && echo "$VIRTUAL_ENV"'],
98-
capture_output=True,
99-
text=True,
100-
cwd=str(work_dir),
101-
encoding="utf-8",
102-
)
103124
assert result.returncode == 0
104125
assert result.stdout.strip() == str(relocated)
105126

106127

128+
@pytest.mark.skipif(IS_WIN, reason="Github Actions ships with WSL bash")
129+
@pytest.mark.parametrize(
130+
"payload",
131+
[
132+
pytest.param("x'$(id > PWNED)'y", id="command-substitution"),
133+
pytest.param("x'`id > PWNED`'y", id="backticks"),
134+
pytest.param("a';id > PWNED;'b", id="statement-separator"),
135+
],
136+
)
137+
def test_bash_activate_relocation_does_not_run_path_commands(
138+
relocated_bash_venv: Callable[[str], tuple[subprocess.CompletedProcess[str], Path, Path]], payload: str
139+
) -> None:
140+
_, _, work_dir = relocated_bash_venv(payload)
141+
142+
assert not (work_dir / "PWNED").exists()
143+
144+
107145
@pytest.mark.skipif(IS_WIN, reason="Github Actions ships with WSL bash")
108146
def test_bash_activate_does_not_export_ps1(tmp_path, current_fastest) -> None:
109147
dest = tmp_path / "env"

‎tests/unit/activation/test_fish.py‎

Lines changed: 48 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,66 +1,84 @@
11
from __future__ import annotations
22

33
import os
4+
import shlex
45
import shutil
56
import subprocess
67
import sys
78
from argparse import Namespace
9+
from types import SimpleNamespace
10+
from typing import TYPE_CHECKING
811

912
import pytest
1013

1114
from virtualenv.activation import FishActivator
1215
from virtualenv.info import IS_WIN
1316

17+
if TYPE_CHECKING:
18+
from collections.abc import Callable
19+
from pathlib import Path
20+
1421
FISH = shutil.which("fish")
1522

1623

24+
@pytest.fixture
25+
def rendered_activate_fish(tmp_path: Path) -> Callable[[str | None, str | None], Path]:
26+
def render(tcl_lib: str | None, tk_lib: str | None) -> Path:
27+
creator = SimpleNamespace(
28+
dest=tmp_path,
29+
bin_dir=tmp_path / "bin",
30+
interpreter=SimpleNamespace(tcl_lib=tcl_lib, tk_lib=tk_lib),
31+
pyenv_cfg={},
32+
env_name="my-env",
33+
)
34+
creator.bin_dir.mkdir()
35+
FishActivator(Namespace(prompt=None)).generate(creator)
36+
return creator.bin_dir / "activate.fish"
37+
38+
return render
39+
40+
1741
@pytest.mark.parametrize(
1842
("tcl_lib", "tk_lib", "present"),
1943
[
2044
("/path/to/tcl", "/path/to/tk", True),
45+
("/Program Files/tcl", "/Program Files/tk", True),
2146
(None, None, False),
2247
],
2348
)
24-
def test_fish_tkinter_generation(tmp_path, tcl_lib, tk_lib, present) -> None:
25-
# GIVEN
26-
class MockInterpreter:
27-
pass
28-
29-
interpreter = MockInterpreter()
30-
interpreter.tcl_lib = tcl_lib
31-
interpreter.tk_lib = tk_lib
32-
33-
class MockCreator:
34-
def __init__(self, dest) -> None:
35-
self.dest = dest
36-
self.bin_dir = dest / "bin"
37-
self.bin_dir.mkdir()
38-
self.interpreter = interpreter
39-
self.pyenv_cfg = {}
40-
self.env_name = "my-env"
41-
42-
creator = MockCreator(tmp_path)
43-
options = Namespace(prompt=None)
44-
activator = FishActivator(options)
45-
46-
# WHEN
47-
activator.generate(creator)
48-
content = (creator.bin_dir / "activate.fish").read_text(encoding="utf-8")
49-
50-
# THEN
51-
# PKG_CONFIG_PATH is always set
49+
def test_fish_tkinter_generation(
50+
rendered_activate_fish: Callable[[str | None, str | None], Path],
51+
tcl_lib: str | None,
52+
tk_lib: str | None,
53+
present: bool,
54+
) -> None:
55+
content = rendered_activate_fish(tcl_lib, tk_lib).read_text(encoding="utf-8")
56+
5257
assert 'set -gx _OLD_PKG_CONFIG_PATH "$PKG_CONFIG_PATH"' in content
5358
assert 'set -gx PKG_CONFIG_PATH "$VIRTUAL_ENV/lib/pkgconfig:$PKG_CONFIG_PATH"' in content
5459
assert "set -e _OLD_PKG_CONFIG_PATH" in content
5560

5661
if present:
57-
assert "set -gx TCL_LIBRARY '/path/to/tcl'" in content
58-
assert "set -gx TK_LIBRARY '/path/to/tk'" in content
62+
assert f"set -gx TCL_LIBRARY {shlex.quote(tcl_lib)}\n" in content
63+
assert f"set -gx TK_LIBRARY {shlex.quote(tk_lib)}\n" in content
5964
else:
6065
assert "if test -n ''\n set -gx _OLD_VIRTUAL_TCL_LIBRARY" in content
6166
assert "if test -n ''\n set -gx _OLD_VIRTUAL_TK_LIBRARY" in content
6267

6368

69+
@pytest.mark.skipif(IS_WIN, reason="fish is not available on Windows")
70+
@pytest.mark.skipif(FISH is None, reason="fish is not installed")
71+
def test_fish_tkinter_path_does_not_run_commands(
72+
rendered_activate_fish: Callable[[str | None, str | None], Path], tmp_path: Path
73+
) -> None:
74+
marker = tmp_path / "PWNED"
75+
script = rendered_activate_fish(f"/tcl/(touch {marker})/lib", "/tk/lib")
76+
77+
subprocess.run([FISH, "-c", f"source '{script}'"], capture_output=True, text=True, timeout=60, check=False)
78+
79+
assert not marker.exists()
80+
81+
6482
@pytest.mark.skipif(IS_WIN, reason="fish is not available on Windows")
6583
@pytest.mark.skipif(FISH is None, reason="fish is not installed")
6684
def test_fish_prompt_survives_shadowed_source(activation_python, tmp_path) -> None:

0 commit comments

Comments
 (0)