🛡️ Sentinel: [CRITICAL] Fix command injection vulnerability via execSync#100
Conversation
Replaced usage of `execSync` with `execFileSync` in scenarios where dynamic strings (such as `chownTo` in terminals.ts or `sudoUser` in sync-unix.ts) were concatenated into shell commands. Since `execSync` executes in a `/bin/sh` shell, it allows command injection if the user-provided variables are manipulated. `execFileSync` avoids this by passing variables directly as arguments to the executable. Co-authored-by: Donach <39565367+Donach@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. |
1 participant
🚨 Severity: CRITICAL
💡 Vulnerability: Command injection in
terminals.ts,sync-unix.ts, andzellij.ts. The code usedexecSyncwith dynamic string interpolation to execute shell commands.🎯 Impact: Attackers could supply manipulated inputs (such as an environment variable or username) to execute arbitrary commands on the system.
🔧 Fix: Replaced
execSyncwithexecFileSyncacross all affected sites. For variables likesudoUser, an array of arguments including--is used to prevent option injection.✅ Verification: Ran test suite and validated correct command execution for
chown,getent passwd, andwhoami.PR created automatically by Jules for task 11576370676561460358 started by @Donach