diff --git a/PR_DESCRIPTION.md b/PR_DESCRIPTION.md new file mode 100644 index 0000000..2df1f51 --- /dev/null +++ b/PR_DESCRIPTION.md @@ -0,0 +1,10 @@ +# 🔒 Security Fix: Replace unsafe `preexec_fn` with `start_new_session` + +🎯 **What:** +Replaced `preexec_fn=os.setsid` with `start_new_session=True` in `subprocess.Popen` within `commando/core/audit.py`. + +⚠️ **Risk:** +Using `preexec_fn` in Python can lead to severe security and stability issues in multi-threaded applications. Specifically, if a thread holds a lock or forks concurrently, running an arbitrary callable via `preexec_fn` just before `exec()` can cause the child process to deadlock, potentially leading to denial of service or unexpected program crashes. This is a well-documented risk in the Python standard library. + +🛡️ **Solution:** +Python 3.2+ introduced the `start_new_session` parameter to `subprocess.Popen` specifically to handle the common use case of running `os.setsid` safely. By switching to `start_new_session=True`, the child process continues to start in a new process group without risking deadlocks from concurrent threading operations, ensuring safe and reliable dynamic auditing (e.g., via `strace`). diff --git a/commando/core/audit.py b/commando/core/audit.py index 8c15c86..d43e958 100644 --- a/commando/core/audit.py +++ b/commando/core/audit.py @@ -74,9 +74,13 @@ def search_command( execute_strace = False else: try: - ans = input( - f"{YELLOW}Warning: '{base_command}' is a custom import. Append '--help' for kinetic audit? (y/N): {RESET}" - ).strip().lower() + ans = ( + input( + f"{YELLOW}Warning: '{base_command}' is a custom import. Append '--help' for kinetic audit? (y/N): {RESET}" + ) + .strip() + .lower() + ) except (EOFError, KeyboardInterrupt): ans = "n" @@ -98,7 +102,7 @@ def search_command( stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, - preexec_fn=os.setsid, + start_new_session=True, ) try: