CVE-2014-0156: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') (CWE-78)
value: Awesome spawn contains OS command injection vulnerability, which allows execution of additional commands passed to Awesome spawn as arguments. If untrusted input was included in command arguments, attacker could use this flaw to execute arbitrary command.
CWE description: The product constructs all or part of an OS command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended OS command when it is sent to a downstream component.
CVSS v3 base score: 9.8
Published: 2022-06-30T21:15Z
Repository: https://github.com/ManageIQ/awesome_spawn
Language: Ruby
Files changed: lib/awesome_spawn.rb, lib/awesome_spawn/command_line_builder.rb, spec/awesome_spawn_spec.rb, spec/command_line_builder_spec.rb
Commit date: 2014-07-03 14:14:51 -0400
Commit message: Separate command line building and sanitizing into its own class.

Diff: diff --git a/lib/awesome_spawn.rb b/lib/awesome_spawn.rb
index 967997a..da1651a 100644
--- a/lib/awesome_spawn.rb
+++ b/lib/awesome_spawn.rb
@@ -1,11 +1,11 @@
 require "awesome_spawn/version"
+require "awesome_spawn/command_line_builder"
 require "awesome_spawn/command_result"
 require "awesome_spawn/command_result_error"
 require "awesome_spawn/no_such_file_error"
 
-require "shellwords"
 require "open3"
 
 module AwesomeSpawn
   extend self
 
@@ -98,64 +98,17 @@ module AwesomeSpawn
     end
 
     command_result
   end
 
-  # Build the full command line.
-  #
-  # @param [String] command The command to run
-  # @param [Hash,Array] params Optional command line parameters. They can
-  #   be passed as a Hash or associative Array. The values are sanitized to
-  #   prevent command line injection.  Keys as symbols are prefixed with `--`,
-  #   and `_` is replaced with `-`.
-  #
-  #   - `{:key => "value"}`            generates `--key value`
-  #   - `{"--key" => "value"}`         generates `--key value`
-  #   - `{:key= => "value"}`           generates `--key=value`
-  #   - `{"--key=" => "value"}`        generates `--key=value`
-  #   - `{:key_name => "value"}`       generates `--key-name value`
-  #   - `{:key => nil}`                generates `--key`
-  #   - `{"-f" => ["file1", "file2"]}` generates `-f file1 file2`
-  #   - `{nil => ["file1", "file2"]}`  generates `file1 file2`
-  #
-  # @return [String] The full command line
+  # (see CommandLineBuilder#build)
   def build_command_line(command, params = nil)
-    return command.to_s if params.nil? || params.empty?
-    "#{command} #{assemble_params(sanitize(params))}"
+    CommandLineBuilder.new.build(command, params)
   end
 
   private
 
-  def sanitize(params)
-    return [] if params.nil? || params.empty?
-    params.collect do |k, v|
-      [sanitize_key(k), sanitize_value(v)]
-    end
-  end
-
-  def sanitize_key(key)
-    case key
-    when Symbol then "--#{key.to_s.tr("_", "-")}"
-    else             key
-    end
-  end
-
-  def sanitize_value(value)
-    case value
-    when Array    then value.collect { |i| i.to_s.shellescape }
-    when NilClass then value
-    else               value.to_s.shellescape
-    end
-  end
-
-  def assemble_params(sanitized_params)
-    sanitized_params.collect do |pair|
-      pair_joiner = pair.first.to_s.end_with?("=") ? "" : " "
-      pair.flatten.compact.join(pair_joiner)
-    end.join(" ")
-  end
-
   def launch(command, in_data, spawn_options = {})
     spawn_options = spawn_options.merge(:stdin_data => in_data) if in_data
     output, error, status = Open3.capture3(command, spawn_options)
     status &&= status.exitstatus
     return output, error, status
diff --git a/lib/awesome_spawn/command_line_builder.rb b/lib/awesome_spawn/command_line_builder.rb
new file mode 100644
index 0000000..55d9099
--- /dev/null
+++ b/lib/awesome_spawn/command_line_builder.rb
@@ -0,0 +1,59 @@
+require "shellwords"
+
+module AwesomeSpawn
+  class CommandLineBuilder
+    # Build the full command line.
+    #
+    # @param [String] command The command to run
+    # @param [Hash,Array] params Optional command line parameters. They can
+    #   be passed as a Hash or associative Array. The values are sanitized to
+    #   prevent command line injection.  Keys as symbols are prefixed with `--`,
+    #   and `_` is replaced with `-`.
+    #
+    #   - `{:key => "value"}`            generates `--key value`
+    #   - `{"--key" => "value"}`         generates `--key value`
+    #   - `{:key= => "value"}`           generates `--key=value`
+    #   - `{"--key=" => "value"}`        generates `--key=value`
+    #   - `{:key_name => "value"}`       generates `--key-name value`
+    #   - `{:key => nil}`                generates `--key`
+    #   - `{"-f" => ["file1", "file2"]}` generates `-f file1 file2`
+    #   - `{nil => ["file1", "file2"]}`  generates `file1 file2`
+    #
+    # @return [String] The full command line
+    def build(command, params = nil)
+      return command.to_s if params.nil? || params.empty?
+      "#{command} #{assemble_params(sanitize(params))}"
+    end
+
+    private
+
+    def sanitize(params)
+      return [] if params.nil? || params.empty?
+      params.collect do |k, v|
+        [sanitize_key(k), sanitize_value(v)]
+      end
+    end
+
+    def sanitize_key(key)
+      case key
+      when Symbol then "--#{key.to_s.tr("_", "-")}"
+      else             key
+      end
+    end
+
+    def sanitize_value(value)
+      case value
+      when Array    then value.collect { |i| i.to_s.shellescape }
+      when NilClass then value
+      else               value.to_s.shellescape
+      end
+    end
+
+    def assemble_params(sanitized_params)
+      sanitized_params.collect do |pair|
+        pair_joiner = pair.first.to_s.end_with?("=") ? "" : " "
+        pair.flatten.compact.join(pair_joiner)
+      end.join(" ")
+    end
+  end
+end
diff --git a/spec/awesome_spawn_spec.rb b/spec/awesome_spawn_spec.rb
index 0988dd2..7da5d87 100644
--- a/spec/awesome_spawn_spec.rb
+++ b/spec/awesome_spawn_spec.rb
@@ -126,73 +126,6 @@ describe AwesomeSpawn do
   context ".run!" do
     include_examples "run" do
       let(:run_method) {"run!"}
     end
   end
-
-  context ".build_command_line" do
-    it "sanitizes crazy params" do
-      cl = subject.build_command_line("true", modified_params)
-      expect(cl).to eq "true --user bob --pass P@\\$sw0\\^\\&\\ \\|\\<\\>/-\\+\\*d\\% --db --desc=Some\\ Description --symkey --symkey-dash pkg1 some\\ pkg --pool 123 --pool 456"
-    end
-
-    it "handles Symbol keys" do
-      cl = subject.build_command_line("true", :abc => "def")
-      expect(cl).to eq "true --abc def"
-    end
-
-    it "handles Symbol keys with tailing '='" do
-      cl = subject.build_command_line("true", :abc= => "def")
-      expect(cl).to eq "true --abc=def"
-    end
-
-    it "handles Symbol keys with underscore" do
-      cl = subject.build_command_line("true", :abc_def => "ghi")
-      expect(cl).to eq "true --abc-def ghi"
-    end
-
-    it "handles Symbol keys with underscore and tailing '='" do
-      cl = subject.build_command_line("true", :abc_def= => "ghi")
-      expect(cl).to eq "true --abc-def=ghi"
-    end
-
-    it "sanitizes Fixnum array param value" do
-      cl = subject.build_command_line("true", nil => [1])
-      expect(cl).to eq "true 1"
-    end
-
-    it "sanitizes Pathname param value" do
-      cl = subject.build_command_line("true", nil => [Pathname.new("/usr/bin/ruby")])
-      expect(cl).to eq "true /usr/bin/ruby"
-    end
-
-    it "sanitizes Pathname param key" do
-      cl = subject.build_command_line("true", Pathname.new("/usr/bin/ruby") => nil)
-      expect(cl).to eq "true /usr/bin/ruby"
-    end
-
-    it "with params as empty Hash" do
-      cl = subject.build_command_line("true", {})
-      expect(cl).to eq "true"
-    end
-
-    it "with params as nil" do
-      cl = subject.build_command_line("true", nil)
-      expect(cl).to eq "true"
-    end
-
-    it "without params" do
-      cl = subject.build_command_line("true")
-      expect(cl).to eq "true"
-    end
-
-    it "with Pathname command" do
-      cl = subject.build_command_line(Pathname.new("/usr/bin/ruby"))
-      expect(cl).to eq "/usr/bin/ruby"
-    end
-
-    it "with Pathname command and params" do
-      cl = subject.build_command_line(Pathname.new("/usr/bin/ruby"), "-v" => nil)
-      expect(cl).to eq "/usr/bin/ruby -v"
-    end
-  end
 end
diff --git a/spec/command_line_builder_spec.rb b/spec/command_line_builder_spec.rb
new file mode 100644
index 0000000..b308619
--- /dev/null
+++ b/spec/command_line_builder_spec.rb
@@ -0,0 +1,88 @@
+require 'spec_helper'
+
+describe AwesomeSpawn::CommandLineBuilder do
+  subject { described_class.new }
+
+  let(:params) do
+    {
+      "--user"     => "bob",
+      "--pass"     => "P@$sw0^& |<>/-+*d%",
+      "--db"       => nil,
+      "--desc="    => "Some Description",
+      :symkey      => nil,
+      :symkey_dash => nil,
+      nil          => ["pkg1", "some pkg"]
+    }
+  end
+
+  let (:modified_params) do
+    params.to_a + [123, 456].collect {|pool| ["--pool", pool]}
+  end
+
+  context "#build" do
+    it "sanitizes crazy params" do
+      cl = subject.build("true", modified_params)
+      expect(cl).to eq "true --user bob --pass P@\\$sw0\\^\\&\\ \\|\\<\\>/-\\+\\*d\\% --db --desc=Some\\ Description --symkey --symkey-dash pkg1 some\\ pkg --pool 123 --pool 456"
+    end
+
+    it "handles Symbol keys" do
+      cl = subject.build("true", :abc => "def")
+      expect(cl).to eq "true --abc def"
+    end
+
+    it "handles Symbol keys with tailing '='" do
+      cl = subject.build("true", :abc= => "def")
+      expect(cl).to eq "true --abc=def"
+    end
+
+    it "handles Symbol keys with underscore" do
+      cl = subject.build("true", :abc_def => "ghi")
+      expect(cl).to eq "true --abc-def ghi"
+    end
+
+    it "handles Symbol keys with underscore and tailing '='" do
+      cl = subject.build("true", :abc_def= => "ghi")
+      expect(cl).to eq "true --abc-def=ghi"
+    end
+
+    it "sanitizes Fixnum array param value" do
+      cl = subject.build("true", nil => [1])
+      expect(cl).to eq "true 1"
+    end
+
+    it "sanitizes Pathname param value" do
+      cl = subject.build("true", nil => [Pathname.new("/usr/bin/ruby")])
+      expect(cl).to eq "true /usr/bin/ruby"
+    end
+
+    it "sanitizes Pathname param key" do
+      cl = subject.build("true", Pathname.new("/usr/bin/ruby") => nil)
+      expect(cl).to eq "true /usr/bin/ruby"
+    end
+
+    it "with params as empty Hash" do
+      cl = subject.build("true", {})
+      expect(cl).to eq "true"
+    end
+
+    it "with params as nil" do
+      cl = subject.build("true", nil)
+      expect(cl).to eq "true"
+    end
+
+    it "without params" do
+      cl = subject.build("true")
+      expect(cl).to eq "true"
+    end
+
+    it "with Pathname command" do
+      cl = subject.build(Pathname.new("/usr/bin/ruby"))
+      expect(cl).to eq "/usr/bin/ruby"
+    end
+
+    it "with Pathname command and params" do
+      cl = subject.build(Pathname.new("/usr/bin/ruby"), "-v" => nil)
+      expect(cl).to eq "/usr/bin/ruby -v"
+    end
+  end
+end

Vulnerable code: require "shellwords"
  # Build the full command line.
  #
  # @param [String] command The command to run
  # @param [Hash,Array] params Optional command line parameters. They can
  #   be passed as a Hash or associative Array. The values are sanitized to
  #   prevent command line injection.  Keys as symbols are prefixed with `--`,
  #   and `_` is replaced with `-`.
  #
  #   - `{:key => "value"}`            generates `--key value`
  #   - `{"--key" => "value"}`         generates `--key value`
  #   - `{:key= => "value"}`           generates `--key=value`
  #   - `{"--key=" => "value"}`        generates `--key=value`
  #   - `{:key_name => "value"}`       generates `--key-name value`
  #   - `{:key => nil}`                generates `--key`
  #   - `{"-f" => ["file1", "file2"]}` generates `-f file1 file2`
  #   - `{nil => ["file1", "file2"]}`  generates `file1 file2`
  #
  # @return [String] The full command line
    return command.to_s if params.nil? || params.empty?
    "#{command} #{assemble_params(sanitize(params))}"
  def sanitize(params)
    return [] if params.nil? || params.empty?
    params.collect do |k, v|
      [sanitize_key(k), sanitize_value(v)]
    end
  end

  def sanitize_key(key)
    case key
    when Symbol then "--#{key.to_s.tr("_", "-")}"
    else             key
    end
  end

  def sanitize_value(value)
    case value
    when Array    then value.collect { |i| i.to_s.shellescape }
    when NilClass then value
    else               value.to_s.shellescape
    end
  end

  def assemble_params(sanitized_params)
    sanitized_params.collect do |pair|
      pair_joiner = pair.first.to_s.end_with?("=") ? "" : " "
      pair.flatten.compact.join(pair_joiner)
    end.join(" ")
  end


  context ".build_command_line" do
    it "sanitizes crazy params" do
      cl = subject.build_command_line("true", modified_params)
      expect(cl).to eq "true --user bob --pass P@\\$sw0\\^\\&\\ \\|\\<\\>/-\\+\\*d\\% --db --desc=Some\\ Description --symkey --symkey-dash pkg1 some\\ pkg --pool 123 --pool 456"
    end

    it "handles Symbol keys" do
      cl = subject.build_command_line("true", :abc => "def")
      expect(cl).to eq "true --abc def"
    end

    it "handles Symbol keys with tailing '='" do
      cl = subject.build_command_line("true", :abc= => "def")
      expect(cl).to eq "true --abc=def"
    end

    it "handles Symbol keys with underscore" do
      cl = subject.build_command_line("true", :abc_def => "ghi")
      expect(cl).to eq "true --abc-def ghi"
    end

    it "handles Symbol keys with underscore and tailing '='" do
      cl = subject.build_command_line("true", :abc_def= => "ghi")
      expect(cl).to eq "true --abc-def=ghi"
    end

    it "sanitizes Fixnum array param value" do
      cl = subject.build_command_line("true", nil => [1])
      expect(cl).to eq "true 1"
    end

    it "sanitizes Pathname param value" do
      cl = subject.build_command_line("true", nil => [Pathname.new("/usr/bin/ruby")])
      expect(cl).to eq "true /usr/bin/ruby"
    end

    it "sanitizes Pathname param key" do
      cl = subject.build_command_line("true", Pathname.new("/usr/bin/ruby") => nil)
      expect(cl).to eq "true /usr/bin/ruby"
    end

    it "with params as empty Hash" do
      cl = subject.build_command_line("true", {})
      expect(cl).to eq "true"
    end

    it "with params as nil" do
      cl = subject.build_command_line("true", nil)
      expect(cl).to eq "true"
    end

    it "without params" do
      cl = subject.build_command_line("true")
      expect(cl).to eq "true"
    end

    it "with Pathname command" do
      cl = subject.build_command_line(Pathname.new("/usr/bin/ruby"))
      expect(cl).to eq "/usr/bin/ruby"
    end

    it "with Pathname command and params" do
      cl = subject.build_command_line(Pathname.new("/usr/bin/ruby"), "-v" => nil)
      expect(cl).to eq "/usr/bin/ruby -v"
    end
  end
Fixed code: require "awesome_spawn/command_line_builder"
  # (see CommandLineBuilder#build)
    CommandLineBuilder.new.build(command, params)
require "shellwords"

module AwesomeSpawn
  class CommandLineBuilder
    # Build the full command line.
    #
    # @param [String] command The command to run
    # @param [Hash,Array] params Optional command line parameters. They can
    #   be passed as a Hash or associative Array. The values are sanitized to
    #   prevent command line injection.  Keys as symbols are prefixed with `--`,
    #   and `_` is replaced with `-`.
    #
    #   - `{:key => "value"}`            generates `--key value`
    #   - `{"--key" => "value"}`         generates `--key value`
    #   - `{:key= => "value"}`           generates `--key=value`
    #   - `{"--key=" => "value"}`        generates `--key=value`
    #   - `{:key_name => "value"}`       generates `--key-name value`
    #   - `{:key => nil}`                generates `--key`
    #   - `{"-f" => ["file1", "file2"]}` generates `-f file1 file2`
    #   - `{nil => ["file1", "file2"]}`  generates `file1 file2`
    #
    # @return [String] The full command line
    def build(command, params = nil)
      return command.to_s if params.nil? || params.empty?
      "#{command} #{assemble_params(sanitize(params))}"
    end

    private

    def sanitize(params)
      return [] if params.nil? || params.empty?
      params.collect do |k, v|
        [sanitize_key(k), sanitize_value(v)]
      end
    end

    def sanitize_key(key)
      case key
      when Symbol then "--#{key.to_s.tr("_", "-")}"
      else             key
      end
    end

    def sanitize_value(value)
      case value
      when Array    then value.collect { |i| i.to_s.shellescape }
      when NilClass then value
      else               value.to_s.shellescape
      end
    end

    def assemble_params(sanitized_params)
      sanitized_params.collect do |pair|
        pair_joiner = pair.first.to_s.end_with?("=") ? "" : " "
        pair.flatten.compact.join(pair_joiner)
      end.join(" ")
    end
  end
end
require 'spec_helper'

describe AwesomeSpawn::CommandLineBuilder do
  subject { described_class.new }

  let(:params) do
    {
      "--user"     => "bob",
      "--pass"     => "P@$sw0^& |<>/-+*d%",
      "--db"       => nil,
      "--desc="    => "Some Description",
      :symkey      => nil,
      :symkey_dash => nil,
      nil          => ["pkg1", "some pkg"]
    }
  end

  let (:modified_params) do
    params.to_a + [123, 456].collect {|pool| ["--pool", pool]}
  end

  context "#build" do
    it "sanitizes crazy params" do
      cl = subject.build("true", modified_params)
      expect(cl).to eq "true --user bob --pass P@\\$sw0\\^\\&\\ \\|\\<\\>/-\\+\\*d\\% --db --desc=Some\\ Description --symkey --symkey-dash pkg1 some\\ pkg --pool 123 --pool 456"
    end

    it "handles Symbol keys" do
      cl = subject.build("true", :abc => "def")
      expect(cl).to eq "true --abc def"
    end

    it "handles Symbol keys with tailing '='" do
      cl = subject.build("true", :abc= => "def")
      expect(cl).to eq "true --abc=def"
    end

    it "handles Symbol keys with underscore" do
      cl = subject.build("true", :abc_def => "ghi")
      expect(cl).to eq "true --abc-def ghi"
    end

    it "handles Symbol keys with underscore and tailing '='" do
      cl = subject.build("true", :abc_def= => "ghi")
      expect(cl).to eq "true --abc-def=ghi"
    end

    it "sanitizes Fixnum array param value" do
      cl = subject.build("true", nil => [1])
      expect(cl).to eq "true 1"
    end

    it "sanitizes Pathname param value" do
      cl = subject.build("true", nil => [Pathname.new("/usr/bin/ruby")])
      expect(cl).to eq "true /usr/bin/ruby"
    end

    it "sanitizes Pathname param key" do
      cl = subject.build("true", Pathname.new("/usr/bin/ruby") => nil)
      expect(cl).to eq "true /usr/bin/ruby"
    end

    it "with params as empty Hash" do
      cl = subject.build("true", {})
      expect(cl).to eq "true"
    end

    it "with params as nil" do
      cl = subject.build("true", nil)
      expect(cl).to eq "true"
    end

    it "without params" do
      cl = subject.build("true")
      expect(cl).to eq "true"
    end

    it "with Pathname command" do
      cl = subject.build(Pathname.new("/usr/bin/ruby"))
      expect(cl).to eq "/usr/bin/ruby"
    end

    it "with Pathname command and params" do
      cl = subject.build(Pathname.new("/usr/bin/ruby"), "-v" => nil)
      expect(cl).to eq "/usr/bin/ruby -v"
    end
  end
end