Skip to content

sshserver.pl: pass command-line to system() safely - #20996

Closed
vszakats wants to merge 3 commits into
curl:masterfrom
vszakats:runt-sys-safe-1
Closed

sshserver.pl: pass command-line to system() safely#20996
vszakats wants to merge 3 commits into
curl:masterfrom
vszakats:runt-sys-safe-1

Conversation

@vszakats

@vszakats vszakats commented Mar 19, 2026

Copy link
Copy Markdown
Member

Found by Codex Security

Follow-up to 89f306a #16781

@vszakats vszakats added the tests label Mar 19, 2026
@vszakats
vszakats marked this pull request as draft March 19, 2026 00:26
@vszakats
vszakats marked this pull request as ready for review March 19, 2026 00:42
@vszakats vszakats changed the title sshserver.pl: pass arguments safely to system() sshserver.pl: pass arguments to system() safely Mar 19, 2026
@vszakats vszakats changed the title sshserver.pl: pass arguments to system() safely sshserver.pl: pass command-line to system() safely Mar 19, 2026
@vszakats
vszakats requested a review from Copilot March 19, 2026 00:43
@vszakats

Copy link
Copy Markdown
Member Author

augment review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens tests/sshserver.pl by switching ssh-keygen invocation from a shell-constructed command string to Perl’s list-form system() call, preventing shell interpretation of arguments (including those influenced by environment variables like CURL_TEST_SSH_KEY_FORMAT).

Changes:

  • Build ssh-keygen optional arguments as an array (@sshkeygenopt) instead of concatenating into a string.
  • Invoke ssh-keygen via list-form system($prog, @args) for both host and client key generation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@augmentcode

augmentcode Bot commented Mar 19, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: Hardens tests/sshserver.pl by avoiding shell-constructed ssh-keygen command lines and using list-form system() calls instead.

Changes: Builds -m <format> options as an argument array (defaulting to PEM for OpenSSH ≥5.6), preventing injection via CURL_TEST_SSH_KEY_FORMAT and improving quoting/whitespace handling when generating test keys.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

@vszakats vszakats closed this in 1509b0c Mar 19, 2026
@vszakats
vszakats deleted the runt-sys-safe-1 branch March 19, 2026 10:28
outcast36 pushed a commit to greearb/curl that referenced this pull request Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants