Bug #14485: For File#path.tainted? and File#to_path.tainted? should match original.tainted?

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 2.3.3p222 (2016-11-21) [x86_64-linux-gnu]
• Backport: 2.3: UNKNOWN, 2.4: UNKNOWN, 2.5: UNKNOWN

[tscheingeld (Terry Scheingeld)]

Problem: if you create a File object using an untainted path, File#path and File#to_path return identical strings except they are tainted. That's counter-intuitive. If the input path has been properly vetted then File should not taint it.

Here's a simple example which produces a security violation:

#!/usr/bin/ruby -w
$SAFE = 1
path = './myfile.txt'
file = File.open(path, 'r')
File.exist?(file.path)

which gives us this error:

./to-path.rb:5:in `exist?': Insecure operation - exist? (SecurityError)
  from ./to-path.rb:5:in `<main>'

In this example, path isn't tainted because it was created in the program. However, file.path, which is an identical string (i.e. not normalized) is tainted.

This issue became a problem in rack/lint. (Not sure how to tell which version.) Lint tries to do some optimizing, but crashes in these lines:

if @body.respond_to?(:to_path)
  assert("The file identified by body.to_path does not exist") {
    ::File.exist? @body.to_path
  }
end

[jeremyevans0 (Jeremy Evans)]

• File file-path-taint.patch file-path-taint.patch added

I agree that File#path should not be tainted unless the path given was tainted. Attached is a patch that fixes the issue

The code to always taint the result was added in a4934a42cbb84b6679912226581c71b435671f55 in 2003 by matz. However, the change wasn't mentioned in the commit message, and it may have been committed by accident.

• Status changed from Open to Closed

Applied in changeset git|1a759bfe5d554c22571d2e6e4e5998cf06a7b98f.

---

Do not always taint the result of File#path

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
a4934a42cbb84b6679912226581c71b435671f55 (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]

[jeremyevans0 (Jeremy Evans)]

• Status changed from Closed to Open

I had to revert my patch for this because it failed on a lot of operating systems. There must be cases where the file path is tainted on those systems other than the one place where I removed rb_obj_taint. Either that, or the new test I wrote for this (which is the only failure), is flawed and needs to be fixed.

• Status changed from Open to Closed

Applied in changeset git|a50bc9f3c8e0696ede25305c03eadecc543b863b.

---

Do not always taint the result of File#path

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
a4934a42cbb84b6679912226581c71b435671f55 (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]