Do not always taint the result of File#path

jeremyevans · jeremyevans · commit 1a759bfe5d55 · 2019-07-29T10:45:14.000-07:00
The result should only be tainted if the path given to the method was tainted. The code to always taint the result was added in a4934a4 (svn revision 4892) in 2003 by matz. However, the change wasn't mentioned in the commit message, and it may have been committed by accident. Skip part of a readline test that uses Reline. Reline in general would pass the test, but Reline's test mode doesn't raise a SecurityError if passing a tainted prompt and $SAFE >= 1. This was hidden earlier because File#path was always returning a tainted string. Fixes [Bug ruby#14485]
diff --git a/file.c b/file.c
@@ -475,7 +475,7 @@ rb_file_path(VALUE obj)
         rb_raise(rb_eIOError, "File is unnamed (TMPFILE?)");
     }
 
-    return rb_obj_taint(rb_str_dup(fptr->pathv));
+    return rb_str_dup(fptr->pathv);
 }
 
 static size_t
diff --git a/test/readline/test_readline.rb b/test/readline/test_readline.rb
@@ -41,6 +41,11 @@ def test_readline
       assert_equal("> ", stdout.read(2))
       assert_equal(1, Readline::HISTORY.length)
       assert_equal("hello", Readline::HISTORY[0])
+
+      # Work around lack of SecurityError in Reline
+      # test mode with tainted prompt
+      return if kind_of?(TestRelineAsReadline)
+
       Thread.start {
         $SAFE = 1
         assert_raise(SecurityError) do
diff --git a/test/ruby/test_file_exhaustive.rb b/test/ruby/test_file_exhaustive.rb
@@ -187,6 +187,22 @@ class << o; self; end.class_eval do
     end
   end
 
+  def test_path_taint
+    [regular_file, utf8_file].each do |file|
+      assert_equal(false, File.open(file) {|f| f.path}.tainted?)
+      assert_equal(true, File.open(file.dup.taint) {|f| f.path}.tainted?)
+      o = Object.new
+      class << o; self; end.class_eval do
+        define_method(:to_path) { file }
+      end
+      assert_equal(false, File.open(o) {|f| f.path}.tainted?)
+      class << o; self; end.class_eval do
+        define_method(:to_path) { file.dup.taint }
+      end
+      assert_equal(true, File.open(o) {|f| f.path}.tainted?)
+    end
+  end
+
   def assert_integer(n)
     assert_kind_of(Integer, n)
   end

Original file line number	Diff line number	Diff line change
`@@ -475,7 +475,7 @@ rb_file_path(VALUE obj)`
`475`	`475`	`rb_raise(rb_eIOError, "File is unnamed (TMPFILE?)");`
`476`	`476`	`}`
`477`	`477`
`478`		`- return rb_obj_taint(rb_str_dup(fptr->pathv));`
	`478`	`+ return rb_str_dup(fptr->pathv);`
`479`	`479`	`}`
`480`	`480`
`481`	`481`	`static size_t`