Project

General

Profile

Bug #14485

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

Added by tscheingeld (Terry Scheingeld) over 1 year ago. Updated 25 days ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
ruby -v:
ruby 2.3.3p222 (2016-11-21) [x86_64-linux-gnu]
[ruby-core:85619]

Description

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

Files

file-path-taint.patch (1.9 KB) file-path-taint.patch jeremyevans0 (Jeremy Evans), 06/20/2019 06:58 PM

Associated revisions

Revision 1a759bfe
Added by jeremyevans (Jeremy Evans) 26 days ago

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]

Revision a50bc9f3
Added by jeremyevans (Jeremy Evans) 25 days ago

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]

History

Updated by jeremyevans0 (Jeremy Evans) 2 months ago

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.

#2

Updated by jeremyevans (Jeremy Evans) 26 days ago

  • 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]

Updated by jeremyevans0 (Jeremy Evans) 26 days ago

  • 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.

#4

Updated by jeremyevans (Jeremy Evans) 25 days ago

  • 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]

Also available in: Atom PDF