aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDavid Rodríguez <deivid.rodriguez@riseup.net>2021-11-13 11:18:54 +0100
committergit <svn-admin@ruby-lang.org>2021-12-07 23:27:59 +0900
commit26303c31f0939d093f88f609c846590ad538114f (patch)
tree2ce7b69f77509de139069f7a349a1fa53a32ccd4
parentbb3f17bd98a652f28b7cccadf08213840e267ad1 (diff)
downloadruby-26303c31f0939d093f88f609c846590ad538114f.tar.gz
[rubygems/rubygems] Pass "--" to git commands to separate positional and optional args
To make sure git uri's specified in Gemfile are never misinterpreted as optional arguments, potentially allowing for local code execution. https://github.com/rubygems/rubygems/commit/90b1ed8b9f
-rw-r--r--lib/bundler/source/git/git_proxy.rb4
-rw-r--r--spec/bundler/bundler/source/git/git_proxy_spec.rb28
2 files changed, 26 insertions, 6 deletions
diff --git a/lib/bundler/source/git/git_proxy.rb b/lib/bundler/source/git/git_proxy.rb
index e37ff8724a..745a7fe118 100644
--- a/lib/bundler/source/git/git_proxy.rb
+++ b/lib/bundler/source/git/git_proxy.rb
@@ -95,12 +95,12 @@ module Bundler
SharedHelpers.filesystem_access(path.dirname) do |p|
FileUtils.mkdir_p(p)
end
- git_retry "clone", configured_uri, path.to_s, "--bare", "--no-hardlinks", "--quiet"
+ git_retry "clone", "--bare", "--no-hardlinks", "--quiet", "--", configured_uri, path.to_s
return unless extra_ref
end
with_path do
- git_retry(*["fetch", "--force", "--quiet", "--tags", configured_uri, "refs/heads/*:refs/heads/*", extra_ref].compact, :dir => path)
+ git_retry(*["fetch", "--force", "--quiet", "--tags", "--", configured_uri, "refs/heads/*:refs/heads/*", extra_ref].compact, :dir => path)
end
end
diff --git a/spec/bundler/bundler/source/git/git_proxy_spec.rb b/spec/bundler/bundler/source/git/git_proxy_spec.rb
index 97f06973cb..cffd72cc3f 100644
--- a/spec/bundler/bundler/source/git/git_proxy_spec.rb
+++ b/spec/bundler/bundler/source/git/git_proxy_spec.rb
@@ -11,21 +11,21 @@ RSpec.describe Bundler::Source::Git::GitProxy do
context "with configured credentials" do
it "adds username and password to URI" do
Bundler.settings.temporary(uri => "u:p") do
- expect(subject).to receive(:git_retry).with("clone", "https://u:p@github.com/rubygems/rubygems.git", any_args)
+ expect(subject).to receive(:git_retry).with("clone", "--bare", "--no-hardlinks", "--quiet", "--", "https://u:p@github.com/rubygems/rubygems.git", path.to_s)
subject.checkout
end
end
it "adds username and password to URI for host" do
Bundler.settings.temporary("github.com" => "u:p") do
- expect(subject).to receive(:git_retry).with("clone", "https://u:p@github.com/rubygems/rubygems.git", any_args)
+ expect(subject).to receive(:git_retry).with("clone", "--bare", "--no-hardlinks", "--quiet", "--", "https://u:p@github.com/rubygems/rubygems.git", path.to_s)
subject.checkout
end
end
it "does not add username and password to mismatched URI" do
Bundler.settings.temporary("https://u:p@github.com/rubygems/rubygems-mismatch.git" => "u:p") do
- expect(subject).to receive(:git_retry).with("clone", uri, any_args)
+ expect(subject).to receive(:git_retry).with("clone", "--bare", "--no-hardlinks", "--quiet", "--", uri, path.to_s)
subject.checkout
end
end
@@ -34,7 +34,7 @@ RSpec.describe Bundler::Source::Git::GitProxy do
Bundler.settings.temporary("github.com" => "u:p") do
original = "https://orig:info@github.com/rubygems/rubygems.git"
subject = described_class.new(Pathname("path"), original, "HEAD")
- expect(subject).to receive(:git_retry).with("clone", original, any_args)
+ expect(subject).to receive(:git_retry).with("clone", "--bare", "--no-hardlinks", "--quiet", "--", original, path.to_s)
subject.checkout
end
end
@@ -148,4 +148,24 @@ RSpec.describe Bundler::Source::Git::GitProxy do
end
end
end
+
+ it "doesn't allow arbitrary code execution through Gemfile uris with a leading dash" do
+ gemfile <<~G
+ gem "poc", git: "-u./pay:load.sh"
+ G
+
+ file = bundled_app("pay:load.sh")
+
+ create_file file, <<~RUBY
+ #!/bin/sh
+
+ touch #{bundled_app("canary")}
+ RUBY
+
+ FileUtils.chmod("+x", file)
+
+ bundle :lock, :raise_on_error => false
+
+ expect(Pathname.new(bundled_app("canary"))).not_to exist
+ end
end