Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Sep 24, 2025

This PR fixes a critical command injection vulnerability in three CLI preparation scripts that use execSync() with template literals containing user-controllable paths.

Vulnerability Details

The vulnerable code pattern was present in:

  • src/cli/prepare-phantom.mjs
  • src/cli/prepare-coinbase.mjs
  • src/cli/prepare-metamask.mjs

Before (vulnerable):

execSync(`chmod +x "${scriptPath}" && "${scriptPath}"`, {
  stdio: "inherit",
  cwd: process.cwd(),
})

This pattern is vulnerable to command injection because if scriptPath contains shell metacharacters (like ; rm -rf /), they would be executed as part of the shell command.

Fix Implementation

After (secure):

const chmodResult = spawnSync('chmod', ['+x', scriptPath], {
  stdio: "inherit",
  cwd: process.cwd(),
})
if (chmodResult.error) {
  throw chmodResult.error
}

const scriptResult = spawnSync(scriptPath, [], {
  stdio: "inherit", 
  cwd: process.cwd(),
})
if (scriptResult.error) {
  throw scriptResult.error
}

Why This Fix Works

  1. Array arguments: spawnSync() with array form treats each argument as a literal string, preventing shell interpretation
  2. No shell expansion: Arguments cannot contain executable shell metacharacters
  3. Separate operations: Split the compound command into two separate, secure calls
  4. Maintained functionality: Same behavior with secure execution
  5. Proper error handling: Individual error checking for each operation

Security Impact

This change eliminates the possibility of command injection attacks while preserving all existing functionality. The CLI scripts will continue to work exactly as before, but are now secure against malicious input in file paths.

Fixes #29.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@joe10832 joe10832 marked this pull request as ready for review September 24, 2025 09:50
Copilot AI review requested due to automatic review settings September 24, 2025 09:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@joe10832
Copy link
Member

/.github/copilot-instructions.md
/.github/instructions/**/*.instructions.md
**/AGENTS.md
/CLAUDE.md
/GEMINI.md

@joe10832 joe10832 merged commit aba8caa into main Sep 24, 2025
3 checks passed
@joe10832
Copy link
Member

/.github/copilot-instructions.md
/.github/instructions/**/*.instructions.md
**/AGENTS.md
/CLAUDE.md
/GEMINI.md

Copilot AI changed the title [WIP] Command injection vulnerability: Using template literals with user-controllable path in shell command. Use array form or properly validate/sanitize the path before execution. Fix command injection vulnerability in CLI preparation scripts Sep 24, 2025
Copilot AI requested a review from joe10832 September 24, 2025 09:55
Copy link
Member

@joe10832 joe10832 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment