1
0
Fork 0
mirror of https://github.com/ruby/ruby.git synced 2022-11-09 12:17:21 -05:00

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.
This commit is contained in:
Nobuyoshi Nakada 2019-10-29 22:39:30 +09:00
parent ad4da86669
commit fee5cde00b
No known key found for this signature in database
GPG key ID: 4BC7D6DF58D8DF60
3 changed files with 64 additions and 72 deletions

View file

@ -5,56 +5,51 @@ require 'tmpdir'
describe "CVE-2018-6914 is resisted by" do describe "CVE-2018-6914 is resisted by" do
before :each do before :each do
@tmpdir = ENV['TMPDIR']
@dir = tmp("CVE-2018-6914") @dir = tmp("CVE-2018-6914")
Dir.mkdir(@dir) Dir.mkdir(@dir)
touch "#{@dir}/bar" ENV['TMPDIR'] = @dir
@dir << '/'
@traversal_path = Array.new(@dir.count('/'), '..').join('/') + @dir + '/'
@traversal_path.delete!(':') if platform_is(:windows)
@tempfile = nil @tempfile = nil
end end
after :each do after :each do
ENV['TMPDIR'] = @tmpdir
@tempfile.close! if @tempfile @tempfile.close! if @tempfile
rm_r @dir rm_r @dir
end end
it "Tempfile.open by deleting separators" do it "Tempfile.open by deleting separators" do
expect = Dir.glob(@traversal_path + '*').size @tempfile = Tempfile.open(['../', 'foo'])
@tempfile = Tempfile.open([@traversal_path, 'foo']) actual = @tempfile.path
actual = Dir.glob(@traversal_path + '*').size File.absolute_path(actual).should.start_with?(@dir)
actual.should == expect
end end
it "Tempfile.new by deleting separators" do it "Tempfile.new by deleting separators" do
expect = Dir.glob(@traversal_path + '*').size @tempfile = Tempfile.new('../foo')
@tempfile = Tempfile.new(@traversal_path + 'foo') actual = @tempfile.path
actual = Dir.glob(@traversal_path + '*').size File.absolute_path(actual).should.start_with?(@dir)
actual.should == expect
end end
it "Tempfile.create by deleting separators" do it "Tempfile.create by deleting separators" do
expect = Dir.glob(@traversal_path + '*').size actual = Tempfile.create('../foo') do |t|
Tempfile.create(@traversal_path + 'foo') do t.path
actual = Dir.glob(@traversal_path + '*').size
actual.should == expect
end end
File.absolute_path(actual).should.start_with?(@dir)
end end
it "Dir.mktmpdir by deleting separators" do it "Dir.mktmpdir by deleting separators" do
expect = Dir.glob(@traversal_path + '*').size actual = Dir.mktmpdir('../foo') do |path|
Dir.mktmpdir(@traversal_path + 'foo') do path
actual = Dir.glob(@traversal_path + '*').size
actual.should == expect
end end
File.absolute_path(actual).should.start_with?(@dir)
end end
it "Dir.mktmpdir with an array by deleting separators" do it "Dir.mktmpdir with an array by deleting separators" do
expect = Dir.glob(@traversal_path + '*').size actual = Dir.mktmpdir(['../', 'foo']) do |path|
Dir.mktmpdir([@traversal_path, 'foo']) do path
actual = Dir.glob(@traversal_path + '*').size
actual.should == expect
end end
File.absolute_path(actual).should.start_with?(@dir)
end end
end end

View file

@ -374,53 +374,43 @@ puts Tempfile.new('foo').path
assert_file.not_exist?(path) assert_file.not_exist?(path)
end end
TRAVERSAL_PATH = Array.new(Dir.pwd.split('/').count, '..').join('/') + Dir.pwd + '/'
def test_open_traversal_dir def test_open_traversal_dir
expect = Dir.glob(TRAVERSAL_PATH + '*').count assert_mktmpdir_traversal do |traversal_path|
t = Tempfile.open([TRAVERSAL_PATH, 'foo']) t = Tempfile.open([traversal_path, 'foo'])
actual = Dir.glob(TRAVERSAL_PATH + '*').count t.path
assert_equal expect, actual ensure
rescue Errno::EINVAL t&.close!
if /mswin|mingw/ =~ RUBY_PLATFORM
assert "ok"
else
raise $!
end end
ensure
t&.close!
end end
def test_new_traversal_dir def test_new_traversal_dir
expect = Dir.glob(TRAVERSAL_PATH + '*').count assert_mktmpdir_traversal do |traversal_path|
t = Tempfile.new(TRAVERSAL_PATH + 'foo') t = Tempfile.new(traversal_path + 'foo')
actual = Dir.glob(TRAVERSAL_PATH + '*').count t.path
assert_equal expect, actual ensure
rescue Errno::EINVAL t&.close!
if /mswin|mingw/ =~ RUBY_PLATFORM
assert "ok"
else
raise $!
end end
ensure
t&.close!
end end
def test_create_traversal_dir def test_create_traversal_dir
expect = Dir.glob(TRAVERSAL_PATH + '*').count assert_mktmpdir_traversal do |traversal_path|
t = Tempfile.create(TRAVERSAL_PATH + 'foo') t = Tempfile.create(traversal_path + 'foo')
actual = Dir.glob(TRAVERSAL_PATH + '*').count t.path
assert_equal expect, actual ensure
rescue Errno::EINVAL if t
if /mswin|mingw/ =~ RUBY_PLATFORM t.close
assert "ok" File.unlink(t.path)
else end
raise $!
end end
ensure end
if t
t.close def assert_mktmpdir_traversal
File.unlink(t.path) 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 end
end end

View file

@ -65,22 +65,29 @@ class TestTmpdir < Test::Unit::TestCase
} }
end end
TRAVERSAL_PATH = Array.new(Dir.pwd.split('/').count, '..').join('/') + Dir.pwd + '/'
TRAVERSAL_PATH.delete!(':') if /mswin|mingw/ =~ RUBY_PLATFORM
def test_mktmpdir_traversal def test_mktmpdir_traversal
expect = Dir.glob(TRAVERSAL_PATH + '*').count assert_mktmpdir_traversal do |traversal_path|
Dir.mktmpdir(TRAVERSAL_PATH + 'foo') do Dir.mktmpdir(traversal_path + 'foo') do |actual|
actual = Dir.glob(TRAVERSAL_PATH + '*').count actual
assert_equal expect, actual end
end end
end end
def test_mktmpdir_traversal_array def test_mktmpdir_traversal_array
expect = Dir.glob(TRAVERSAL_PATH + '*').count assert_mktmpdir_traversal do |traversal_path|
Dir.mktmpdir([TRAVERSAL_PATH, 'foo']) do Dir.mktmpdir([traversal_path, 'foo']) do |actual|
actual = Dir.glob(TRAVERSAL_PATH + '*').count actual
assert_equal expect, 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 end
end end