Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Introduce shell metacharacter escaping for exec #491

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

servusdei2018
Copy link

In order to mitigate potential security vulnerabilities arising from shell injection attacks, this PR introduces a function to escape shell metacharacters which may be present in command-line arguments.

It's worth noting that two potential vulnerabilities still exist in

cmdline := fmt.Sprintf("%v setup %v %v\n", o.runtime, o.runtimeArgs, toolkitDir)
//nolint:gosec // TODO: Can we harden this so that there is less risk of command injection
cmd := exec.Command("sh", "-c", cmdline)
and
cmdline := fmt.Sprintf("%v cleanup %v %v\n", o.runtime, o.runtimeArgs, toolkitDir)
//nolint:gosec // TODO: Can we harden this so that there is less risk of command injection
cmd := exec.Command("sh", "-c", cmdline)
where the string o.runtimeArgs is directly interpolated into cmdline, leaving it susceptible to injection attacks. To address this, a more comprehensive solution would involve reimplementing o.runtimeArgs as []string, allowing for proper sanitization using the introduced oci.Escape() function; however, this likely involves a breaking change.

Signed-off-by: Nathanael Bracy <nate@bracy.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant