summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNobuyoshi Nakada <nobu@ruby-lang.org>2019-10-29 22:39:30 +0900
committerNobuyoshi Nakada <nobu@ruby-lang.org>2019-10-29 22:40:41 +0900
commitfee5cde00be7342dc6c00d0b0a0276d09e5252e3 (patch)
treefd80f86afb77cfe29d90013adc6545d47c9518e2
parentad4da86669454dee86844b3e0a3ecf9177084db3 (diff)
Fix tests for CVE-2018-6914
Since the current working directory is not involved in `Tempfile` and `Dir.mktmpdir` (except for the last resort), it is incorrect to derive the traversal path from it. Also, since the rubyspec temporary directory is created under the build directory, this is not involved in the target method. Fixed sporadic errors in test-spec.
-rw-r--r--spec/ruby/security/cve_2018_6914_spec.rb43
-rw-r--r--test/test_tempfile.rb64
-rw-r--r--test/test_tmpdir.rb29
3 files changed, 64 insertions, 72 deletions
diff --git a/spec/ruby/security/cve_2018_6914_spec.rb b/spec/ruby/security/cve_2018_6914_spec.rb
index 1eab3b84cc..dc2f2cd095 100644
--- a/spec/ruby/security/cve_2018_6914_spec.rb
+++ b/spec/ruby/security/cve_2018_6914_spec.rb
@@ -5,56 +5,51 @@ require 'tmpdir'
describe "CVE-2018-6914 is resisted by" do
before :each do
+ @tmpdir = ENV['TMPDIR']
@dir = tmp("CVE-2018-6914")
Dir.mkdir(@dir)
- touch "#{@dir}/bar"
-
- @traversal_path = Array.new(@dir.count('/'), '..').join('/') + @dir + '/'
- @traversal_path.delete!(':') if platform_is(:windows)
+ ENV['TMPDIR'] = @dir
+ @dir << '/'
@tempfile = nil
end
after :each do
+ ENV['TMPDIR'] = @tmpdir
@tempfile.close! if @tempfile
rm_r @dir
end
it "Tempfile.open by deleting separators" do
- expect = Dir.glob(@traversal_path + '*').size
- @tempfile = Tempfile.open([@traversal_path, 'foo'])
- actual = Dir.glob(@traversal_path + '*').size
- actual.should == expect
+ @tempfile = Tempfile.open(['../', 'foo'])
+ actual = @tempfile.path
+ File.absolute_path(actual).should.start_with?(@dir)
end
it "Tempfile.new by deleting separators" do
- expect = Dir.glob(@traversal_path + '*').size
- @tempfile = Tempfile.new(@traversal_path + 'foo')
- actual = Dir.glob(@traversal_path + '*').size
- actual.should == expect
+ @tempfile = Tempfile.new('../foo')
+ actual = @tempfile.path
+ File.absolute_path(actual).should.start_with?(@dir)
end
it "Tempfile.create by deleting separators" do
- expect = Dir.glob(@traversal_path + '*').size
- Tempfile.create(@traversal_path + 'foo') do
- actual = Dir.glob(@traversal_path + '*').size
- actual.should == expect
+ actual = Tempfile.create('../foo') do |t|
+ t.path
end
+ File.absolute_path(actual).should.start_with?(@dir)
end
it "Dir.mktmpdir by deleting separators" do
- expect = Dir.glob(@traversal_path + '*').size
- Dir.mktmpdir(@traversal_path + 'foo') do
- actual = Dir.glob(@traversal_path + '*').size
- actual.should == expect
+ actual = Dir.mktmpdir('../foo') do |path|
+ path
end
+ File.absolute_path(actual).should.start_with?(@dir)
end
it "Dir.mktmpdir with an array by deleting separators" do
- expect = Dir.glob(@traversal_path + '*').size
- Dir.mktmpdir([@traversal_path, 'foo']) do
- actual = Dir.glob(@traversal_path + '*').size
- actual.should == expect
+ actual = Dir.mktmpdir(['../', 'foo']) do |path|
+ path
end
+ File.absolute_path(actual).should.start_with?(@dir)
end
end
diff --git a/test/test_tempfile.rb b/test/test_tempfile.rb
index 8fcba3aea8..7c911a1bf7 100644
--- a/test/test_tempfile.rb
+++ b/test/test_tempfile.rb
@@ -374,53 +374,43 @@ puts Tempfile.new('foo').path
assert_file.not_exist?(path)
end
- TRAVERSAL_PATH = Array.new(Dir.pwd.split('/').count, '..').join('/') + Dir.pwd + '/'
-
def test_open_traversal_dir
- expect = Dir.glob(TRAVERSAL_PATH + '*').count
- t = Tempfile.open([TRAVERSAL_PATH, 'foo'])
- actual = Dir.glob(TRAVERSAL_PATH + '*').count
- assert_equal expect, actual
- rescue Errno::EINVAL
- if /mswin|mingw/ =~ RUBY_PLATFORM
- assert "ok"
- else
- raise $!
+ assert_mktmpdir_traversal do |traversal_path|
+ t = Tempfile.open([traversal_path, 'foo'])
+ t.path
+ ensure
+ t&.close!
end
- ensure
- t&.close!
end
def test_new_traversal_dir
- expect = Dir.glob(TRAVERSAL_PATH + '*').count
- t = Tempfile.new(TRAVERSAL_PATH + 'foo')
- actual = Dir.glob(TRAVERSAL_PATH + '*').count
- assert_equal expect, actual
- rescue Errno::EINVAL
- if /mswin|mingw/ =~ RUBY_PLATFORM
- assert "ok"
- else
- raise $!
+ assert_mktmpdir_traversal do |traversal_path|
+ t = Tempfile.new(traversal_path + 'foo')
+ t.path
+ ensure
+ t&.close!
end
- ensure
- t&.close!
end
def test_create_traversal_dir
- expect = Dir.glob(TRAVERSAL_PATH + '*').count
- t = Tempfile.create(TRAVERSAL_PATH + 'foo')
- actual = Dir.glob(TRAVERSAL_PATH + '*').count
- assert_equal expect, actual
- rescue Errno::EINVAL
- if /mswin|mingw/ =~ RUBY_PLATFORM
- assert "ok"
- else
- raise $!
+ assert_mktmpdir_traversal do |traversal_path|
+ t = Tempfile.create(traversal_path + 'foo')
+ t.path
+ ensure
+ if t
+ t.close
+ File.unlink(t.path)
+ end
end
- ensure
- if t
- t.close
- File.unlink(t.path)
+ end
+
+ def assert_mktmpdir_traversal
+ Dir.mktmpdir do |target|
+ target = target.chomp('/') + '/'
+ traversal_path = target.sub(/\A\w:/, '') # for DOSISH
+ traversal_path = Array.new(target.count('/')-2, '..').join('/') + traversal_path
+ actual = yield traversal_path
+ assert_not_send([File.absolute_path(actual), :start_with?, target])
end
end
end
diff --git a/test/test_tmpdir.rb b/test/test_tmpdir.rb
index 1e633d233b..42bcbc00a8 100644
--- a/test/test_tmpdir.rb
+++ b/test/test_tmpdir.rb
@@ -65,22 +65,29 @@ class TestTmpdir < Test::Unit::TestCase
}
end
- TRAVERSAL_PATH = Array.new(Dir.pwd.split('/').count, '..').join('/') + Dir.pwd + '/'
- TRAVERSAL_PATH.delete!(':') if /mswin|mingw/ =~ RUBY_PLATFORM
-
def test_mktmpdir_traversal
- expect = Dir.glob(TRAVERSAL_PATH + '*').count
- Dir.mktmpdir(TRAVERSAL_PATH + 'foo') do
- actual = Dir.glob(TRAVERSAL_PATH + '*').count
- assert_equal expect, actual
+ assert_mktmpdir_traversal do |traversal_path|
+ Dir.mktmpdir(traversal_path + 'foo') do |actual|
+ actual
+ end
end
end
def test_mktmpdir_traversal_array
- expect = Dir.glob(TRAVERSAL_PATH + '*').count
- Dir.mktmpdir([TRAVERSAL_PATH, 'foo']) do
- actual = Dir.glob(TRAVERSAL_PATH + '*').count
- assert_equal expect, actual
+ assert_mktmpdir_traversal do |traversal_path|
+ Dir.mktmpdir([traversal_path, 'foo']) do |actual|
+ actual
+ end
+ end
+ end
+
+ def assert_mktmpdir_traversal
+ Dir.mktmpdir do |target|
+ target = target.chomp('/') + '/'
+ traversal_path = target.sub(/\A\w:/, '') # for DOSISH
+ traversal_path = Array.new(target.count('/')-2, '..').join('/') + traversal_path
+ actual = yield traversal_path
+ assert_not_send([File.absolute_path(actual), :start_with?, target])
end
end
end