Skip to content

Conserve memory when resolving paths, recursively handle symlinks, check methods now more verbose and granular. #18

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

Open
LucasFaudman wants to merge 20 commits into
base: main
Choose a base branch
from
Open

Conserve memory when resolving paths, recursively handle symlinks, check methods now more verbose and granular. #18

Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
562851d
Fixed all command injection vulnerabilites present in previous implem…
Feb 10, 2024
0d9a0e2
Correctly handle $IFS/${IFS} in commands. check_banned_executable can…
Feb 13, 2024
ffd0425
Tests now check err.value.args[0].startswith(<Expected Exception Mess…
Feb 15, 2024
3d3dab4
Convert to Pytest. Add shell expansion tests, nested shell syntax tes…
Feb 15, 2024
d447303
Correctly handle all shell expansions. Correctly handled deeply neste…
Feb 15, 2024
d5d9974
Convert to Pytest. Add shell expansion tests, nested shell syntax tes…
Feb 15, 2024
73d9c3c
Handle all shell redirection operators
Feb 15, 2024
6a18137
handle arithmetic expansion of bracket paramters and nested expansions
Feb 16, 2024
d22c6fb
- Complete Shell param/brace/sequence expansion for all ALLOWED_SHELL…
Feb 23, 2024
d078eb0
Remove redundant rmtree, remove script* and add time to BANNED_COMMAN…
Feb 27, 2024
be243f3
- check() now uses Popen kwargs to determine the initial env state an…
Feb 27, 2024
c735164
Remove unused os.environ import left by mistake
Feb 27, 2024
c774f1a
Add FuzzDB license.
Mar 5, 2024
ddb99ac
remove unnessary list conversion
Mar 5, 2024
de99993
- Optimized space complexity of command parsing and path resolution b…
Mar 6, 2024
8c12b66
cleanup comments
Mar 7, 2024
a8921a6
Improve readability. (Underlying check logic unchanged)
Mar 7, 2024
21b9ded
Same check logic but more verbose readable code.
Mar 7, 2024
743e313
- Generators used when resolving paths to avoid storing paths all in …
Mar 12, 2024
939293e
Fix sonarcloud Intentionality & Consistency issues from last PR
Mar 13, 2024
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
Prev Previous commit
Next Next commit
cleanup comments
  • Loading branch information
Lucas Faudman Lucas Faudman
Lucas Faudman authored and Lucas Faudman committed Mar 7, 2024
commit 8c12b667c4b8343493e7fab0355aa07fee99b747
19 changes: 9 additions & 10 deletions src/security/safe_command/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,8 @@ def _recursive_resolve_symlinks(path: Path, rglob_dirs: bool = True) -> Iterator
for file in path.rglob("*"):
yield from _recursive_resolve_symlinks(file, rglob_dirs)
else:
# a final .resolve is needed to handle files like /private/etc/passwd on MacOS which behave like symlinks but are not according to Path.is_symlink
# a final .resolve is needed to handle files like /etc/passwd on MacOS which behaves
# like a symlink to /private/etc/passwd but is not a symlink according to Path.is_symlink
yield path.resolve()


Expand Down Expand Up @@ -520,6 +521,7 @@ def check(command: ValidCommand,
max_resolved_paths: int = 10000,
rglob_dirs: bool = True,
**Popen_kwargs) -> None:

if not restrictions:
# No restrictions no checks
return None
Expand Down Expand Up @@ -547,16 +549,14 @@ def check(command: ValidCommand,
# Create local bools for each restriction to avoid membership check each iteration
prevent_uncommon_path_types = "PREVENT_UNCOMMON_PATH_TYPES" in restrictions
prevent_admin_owned_files = "PREVENT_ADMIN_OWNED_FILES" in restrictions

# Then check the parsed command parts for the restrictions if the expanded command passes
parsed_command = _recursive_shlex_split(expanded_command)


# Determine if path resolution is needed based on the restrictions
check_path_string = prevent_sensitive_files or prevent_common_exploit_executables
resolve_paths = check_path_string or prevent_uncommon_path_types or prevent_admin_owned_files


# Then check the parsed command cmd_parts for the restrictions if the expanded command passes checks
num_resolved_paths = 0
for cmd_part in parsed_command:
for cmd_part in _recursive_shlex_split(expanded_command):
if prevent_command_chaining:
check_multiple_commands(cmd_part)

Expand All @@ -576,7 +576,7 @@ def check(command: ValidCommand,
if prevent_common_exploit_executables:
check_path_is_banned_executable(path_string)

if not executable_path and num_resolved_paths == 1:
if not executable_path:
# If the executable is not set by the Popen kwargs it is the first command part (args). see subprocess.py line 1596
executable_path = path
# continue to avoid blocking the executable itself since most are symlinks to the actual executable and owned by root with group wheel or sudo
Expand All @@ -599,8 +599,7 @@ def check_newline_in_expanded_command(expanded_command: str) -> None:

def check_sensitive_files_in_expanded_command(expanded_command: str) -> None:
for sensitive_path in SENSITIVE_FILE_PATHS:
# First check the absolute path strings for the sensitive files
# Then handle edge cases when a sensitive file is part of a command but the path could not be resolved
# Handles edge cases when a sensitive file is part of a command but the path could not be resolved
if sensitive_path in expanded_command:
raise SecurityException(
f"Disallowed access to sensitive file: {sensitive_path}")
Expand Down