🔒 Fix security vulnerabilities in display_switch.py#15
🔒 Fix security vulnerabilities in display_switch.py#15
Conversation
…lay_switch.py - Replaced hardcoded /tmp paths with secure, user-specific paths (prioritizing XDG_RUNTIME_DIR). - Implemented secure file creation using os.open with os.O_EXCL and restrictive permissions (0o600) to prevent symlink attacks and unauthorized access. - Used shlex.join for safe command construction to prevent command injection. - Updated restore_state to verify file ownership and permissions before execution. - Replaced shell=True in subprocess.run with explicit /bin/bash execution. - Added a new security test suite to verify these improvements and prevent regressions. Co-authored-by: tstapler <3860386+tstapler@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
2 participants
This security fix addresses vulnerabilities in the
display-switchutility. The primary issues were:/tmpwith insecure permissions, making them susceptible to symlink attacks and tampering by other users.xrandrwere not properly escaped before being written to a shell script, allowing for potential command injection.shell=True, which is generally discouraged for security reasons.Solution:
XDG_RUNTIME_DIR(typically/run/user/UID) which is private to the user.os.O_EXCLandos.O_CREATduringos.opento ensure the file is created only if it doesn't exist, preventing symlink race conditions. Permissions are set to0o600(read/write only by owner).shlex.jointo correctly escape all command-line arguments, neutralizing potential injection attacks.restore_statenow performs ownership and permission checks before execution and calls/bin/bashdirectly without using a shell.A new test file
test_display_switch_security.pyhas been added to verify these security measures. All existing tests also pass.PR created automatically by Jules for task 10610515049364856440 started by @tstapler