Skip to content

Avoid shell injection passing any array to Open3#capture3

Peter Leitzen requested to merge pl-refactor-open3 into main

It's more secure to pass an array of arguments to avoid shell injection.

Example:

>> require "open3"
=> true
>> dir = ".; id"
=> ".; id"
>> Open3.capture3("ls #{dir}")
=>
["bin\nbuild\ndevfile.gemspec\next\nGemfile\nGemfile.lock\nlib\nMakefile\nREADME.md\nspec\ntmp\nuid=1000(peter) gid=1000(peter) groups=1000(peter),27(sudo),998(docker)\n",
 "",
 #<Process::Status: pid 339530 exit 0>]
>> Open3.capture3({}, "ls", dir)
=> ["", "ls: cannot access '.; id': No such file or directory\n", #<Process::Status: pid 339539 exit 2>]

Not sure if this is exploitable within Devfile::Parser but still good practice.

Related to https://gitlab.com/gitlab-com/gl-security/appsec/appsec-reviews/-/issues/201.

Context

This gem will be introduced to GitLab once gitlab-org/gitlab!105783 (merged) is merged.

Edited by Peter Leitzen

Merge request reports