Merge pull request #9595 from ruby/strengthen-installer-validation

Add executables and bindir validation to the gem installer

Author: hsbt

lib/rubygems/installer.rb

@@ -712,6 +712,18 @@ def verify_spec
     if spec.dependencies.any? {|dep| dep.name =~ /(?:\R|[<>])/ }
       raise Gem::InstallError, "#{spec} has an invalid dependencies"
     end
+
+    if spec.executables.any? {|name| !name.is_a?(String) || name != File.basename(name) || /\A\.\.?\z|\R/.match?(name) }
+      raise Gem::InstallError, "#{spec} has an invalid executable"
+    end
+
+    raise Gem::InstallError, "#{spec} has an invalid bindir" unless spec.bindir.is_a?(String)
+
+    expanded_gem_dir = File.expand_path(gem_dir)
+    expanded_bindir = File.expand_path(File.join(gem_dir, spec.bindir))
+    unless expanded_bindir == expanded_gem_dir || expanded_bindir.start_with?("#{expanded_gem_dir}/")
+      raise Gem::InstallError, "#{spec} has an invalid bindir"
+    end
   end
 
   ##
@@ -720,6 +732,7 @@ def verify_spec
   def app_script_text(bin_file_name)
     # NOTE: that the `load` lines cannot be indented, as old RG versions match
     # against the beginning of the line
+    escaped_bin_file_name = bin_file_name.gsub(/[\\']/) {|c| "\\#{c}" }
     <<~TEXT
       #{shebang bin_file_name}
       #
@@ -743,9 +756,9 @@ def app_script_text(bin_file_name)
       end
 
       if Gem.respond_to?(:activate_and_load_bin_path)
-        Gem.activate_and_load_bin_path('#{spec.name}', '#{bin_file_name}', version)
+        Gem.activate_and_load_bin_path('#{spec.name}', '#{escaped_bin_file_name}', version)
       else
-        load Gem.activate_bin_path('#{spec.name}', '#{bin_file_name}', version)
+        load Gem.activate_bin_path('#{spec.name}', '#{escaped_bin_file_name}', version)
       end
     TEXT
   end

test/rubygems/test_gem_installer.rb

@@ -60,6 +60,21 @@ def test_app_script_text
     end
   end
 
+  def test_app_script_text_escapes_executable_name
+    installer = setup_base_installer
+
+    malicious = "evil');system('id');#"
+    @spec.bindir = "bin"
+    write_file @spec.bin_file(malicious) do |io|
+      io.puts "#!/usr/bin/ruby"
+    end
+
+    wrapper = installer.app_script_text malicious
+
+    assert_includes wrapper, %q{Gem.activate_and_load_bin_path('a', 'evil\');system(\'id\');#', version)}
+    assert_includes wrapper, %q{load Gem.activate_bin_path('a', 'evil\');system(\'id\');#', version)}
+  end
+
   def test_check_executable_overwrite
     installer = setup_base_installer
 
@@ -1954,6 +1969,82 @@ def spec.validate(*args); end
     end
   end
 
+  def test_pre_install_checks_malicious_executables_before_eval
+    spec = util_spec "malicious", "1"
+    def spec.full_name # so the spec is buildable
+      "malicious-1"
+    end
+
+    def spec.validate(*args); end
+    spec.executables = ["../../../tmp/malicious"]
+
+    util_build_gem spec
+
+    gem = File.join(@gemhome, "cache", spec.file_name)
+
+    use_ui @ui do
+      installer = Gem::Installer.at gem
+      e = assert_raise Gem::InstallError do
+        installer.pre_install_checks
+      end
+      assert_equal "#<Gem::Specification name=malicious version=1> has an invalid executable", e.message
+    end
+  end
+
+  def test_pre_install_checks_malicious_bindir_before_eval
+    spec = util_spec "malicious", "1"
+    def spec.full_name # so the spec is buildable
+      "malicious-1"
+    end
+
+    def spec.validate(*args); end
+    spec.bindir = "../../../tmp/malicious"
+
+    util_build_gem spec
+
+    gem = File.join(@gemhome, "cache", spec.file_name)
+
+    use_ui @ui do
+      installer = Gem::Installer.at gem
+      e = assert_raise Gem::InstallError do
+        installer.pre_install_checks
+      end
+      assert_equal "#<Gem::Specification name=malicious version=1> has an invalid bindir", e.message
+    end
+  end
+
+  def test_pre_install_checks_non_string_executable
+    spec = util_spec "malicious", "1"
+    def spec.validate(*args); end
+    spec.executables = [nil]
+
+    installer = Gem::Installer.for_spec spec
+    installer.gem_home = @gemhome
+
+    use_ui @ui do
+      e = assert_raise Gem::InstallError do
+        installer.pre_install_checks
+      end
+      assert_equal "#<Gem::Specification name=malicious version=1> has an invalid executable", e.message
+    end
+  end
+
+  def test_pre_install_checks_non_string_bindir
+    spec = util_spec "malicious", "1"
+    def spec.validate(*args); end
+    spec.bindir = true
+
+    installer = Gem::Installer.for_spec spec
+    installer.gem_home = @gemhome
+
+    use_ui @ui do
+      e = assert_raise Gem::InstallError do
+        installer.pre_install_checks
+      end
+      assert_equal "#<Gem::Specification name=malicious version=1> has an invalid bindir", e.message
+    end
+  end
+
   def test_pre_install_checks_malicious_platform_before_eval
     gem_with_ill_formatted_platform = File.expand_path("packages/ill-formatted-platform-1.0.0.10.gem", __dir__)