https://git.ruby-lang.org/ruby.git/commit/?id=fee5cde00b

From fee5cde00be7342dc6c00d0b0a0276d09e5252e3 Mon Sep 17 00:00:00 2001
From: Nobuyoshi Nakada <nobu@r...>
Date: Tue, 29 Oct 2019 22:39:30 +0900
Subject: 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.

diff --git a/spec/ruby/security/cve_2018_6914_spec.rb b/spec/ruby/security/cve_2018_6914_spec.rb
index 1eab3b8..dc2f2cd 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' https://github.com/ruby/ruby/blob/trunk/spec/ruby/security/cve_2018_6914_spec.rb#L5
 
 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 8fcba3a..7c911a1 100644
--- a/test/test_tempfile.rb
+++ b/test/test_tempfile.rb
@@ -374,53 +374,43 @@ puts Tempfile.new('foo').path https://github.com/ruby/ruby/blob/trunk/test/test_tempfile.rb#L374
     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 1e633d2..42bcbc0 100644
--- a/test/test_tmpdir.rb
+++ b/test/test_tmpdir.rb
@@ -65,22 +65,29 @@ class TestTmpdir < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/test_tmpdir.rb#L65
     }
   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
-- 
cgit v0.10.2


--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/