Skip to content

Commit ed3fae1

Browse files
committed
Escape all knife wrapper arguments to prevent shell command injection
The org/user management commands in wrap-knife.rb build a knife command as a single string that is run through a shell via run_command. Each argument was passed through Shellwords.escape EXCEPT arguments containing '=', which were concatenated verbatim: if arg.include?('=') arg else Shellwords.escape(arg) end The '=' carve-out was intended to preserve "key=value" config options, but it lets any argument containing '=' inject arbitrary shell syntax. For example a field value such as "x=y; <command>" would have its metacharacters interpreted by the shell instead of being passed to knife as a literal argument. Escape every argument unconditionally. Shellwords.escape leaves benign characters such as '=' untouched, so legitimate "key=value" options continue to work while shell metacharacters are neutralized. Signed-off-by: Tim Smith <tsmith84@proton.me>
1 parent b28ea69 commit ed3fae1

1 file changed

Lines changed: 6 additions & 9 deletions

File tree

src/chef-server-ctl/plugins/wrap-knife.rb

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -117,16 +117,13 @@ def self.parse_args(args)
117117
auth_args << "-c" << knife_config
118118
# auth_args << "--config-option" << "ssl_verify_mode=verify_none"
119119

120-
# Build command args - don't escape config options with = signs
120+
# Build command args. Escape every argument before it is interpolated
121+
# into the shell command string. Shellwords.escape leaves benign
122+
# characters (including "=") untouched, so "key=value" style config
123+
# options still pass through unchanged, while shell metacharacters in
124+
# any argument are neutralized rather than executed.
121125
all_args = transformed_args + auth_args
122-
escaped_args = all_args.map do |arg|
123-
# Don't escape arguments that contain = (like config options)
124-
if arg.include?('=')
125-
arg
126-
else
127-
Shellwords.escape(arg)
128-
end
129-
end.join(" ")
126+
escaped_args = all_args.map { |arg| Shellwords.escape(arg) }.join(" ")
130127

131128
knife_command = "#{knife_cmd} #{opc_noun} #{opc_cmd} #{escaped_args}"
132129
status = run_command(knife_command)

0 commit comments

Comments
 (0)