Skip to content

🛡️ Sentinel: [HIGH] Fix command injection in directory picker #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
devdudumuniz wants to merge 1 commit into from
Closed

🛡️ Sentinel: [HIGH] Fix command injection in directory picker #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .jules/sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
## 2024-08-29 - Command Injection in Native Directory Picker

**Vulnerability:** A command injection vulnerability existed in `pysus/web/pages/1_client.py` within the `_native_dir_picker` function. When opening a directory picker on Windows or macOS (Darwin), user-provided strings (`title` and `initialdir`) were directly formatted into PowerShell and AppleScript strings without any escaping.

**Learning:** When using Python's `subprocess.run` to execute scripts that dynamically construct logic via format strings (f-strings) inside platforms like PowerShell or `osascript`, quoting strings within those formats is insufficient if the external platform is processing the final string payload. Even though `shell=True` was not used, the script engines evaluated the unescaped inputs as code.

**Prevention:** Always escape data embedded inside dynamically generated scripts being passed to an interpreter like PowerShell (`'` to `''`) or AppleScript (`\` to `\\`, `"` to `\"`), or pass the data as parameters/arguments to the script rather than embedding them directly in the script source.
12 changes: 9 additions & 3 deletions pysus/web/pages/1_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -726,11 +726,14 @@ def _native_dir_picker(title: str, initialdir: str) -> str:
continue

elif system == "Windows":
# Escape single quotes for PowerShell string literals
title_esc = title.replace("'", "''")
initialdir_esc = initialdir.replace("'", "''")
ps = f"""
Add-Type -AssemblyName System.Windows.Forms
$f = New-Object System.Windows.Forms.FolderBrowserDialog
$f.Description = '{title}'
$f.SelectedPath = '{initialdir}'
$f.Description = '{title_esc}'
$f.SelectedPath = '{initialdir_esc}'
$f.ShowDialog() | Out-Null
$f.SelectedPath
"""
Expand All @@ -742,10 +745,13 @@ def _native_dir_picker(title: str, initialdir: str) -> str:
return r.stdout.strip()

elif system == "Darwin":
# Escape backslashes and double quotes for AppleScript string literals
title_esc = title.replace("\\", "\\\\").replace('"', '\\"')
initialdir_esc = initialdir.replace("\\", "\\\\").replace('"', '\\"')
prompt_line = (
'set f to choose folder with prompt "{}"'
' default location POSIX file "{}"'
).format(title, initialdir)
).format(title_esc, initialdir_esc)
applescript = (
f'tell application "System Events"\n'
f" activate\n"
Expand Down