Skip to content

Commit

Permalink
Do not always taint the result of File#path
Browse files Browse the repository at this point in the history
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 #14485]
  • Loading branch information
jeremyevans committed Jul 29, 2019
1 parent aa97410 commit 1a759bf
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 1 deletion.
2 changes: 1 addition & 1 deletion file.c
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions test/readline/test_readline.rb
Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions test/ruby/test_file_exhaustive.rb
Expand Up @@ -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
Expand Down

0 comments on commit 1a759bf

Please sign in to comment.