Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DOC] Enhanced RDoc for FileUtils #78

Merged
merged 9 commits into from
Jun 6, 2022
Merged

[DOC] Enhanced RDoc for FileUtils #78

Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4c67c3e
Enhanced RDoc for FileUtils
BurdetteLamar May 30, 2022
65f22f4
Enhanced RDoc
BurdetteLamar Jun 1, 2022
8e653bd
Enhanced RDoc
BurdetteLamar Jun 1, 2022
71672b4
Enhanced RDoc
BurdetteLamar Jun 2, 2022
fabcf44
Enhanced RDoc
BurdetteLamar Jun 2, 2022
cec2e7b
Enhanced RDoc
BurdetteLamar Jun 3, 2022
4a9a0f8
Enhanced RDoc
BurdetteLamar Jun 4, 2022
d7fd008
Enhanced RDoc
BurdetteLamar Jun 4, 2022
5163525
Enhanced RDoc
BurdetteLamar Jun 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
211 changes: 147 additions & 64 deletions lib/fileutils.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,61 @@
# files/directories. This equates to passing the <tt>:noop</tt> and
# <tt>:verbose</tt> flags to methods in FileUtils.
#
# == Avoiding the TOCTTOU Vulnerability
#
# For certain methods that recursively remove entries,
# there is a potential vulnerability called the
# {Time-of-check to time-of-use}[https://en.wikipedia.org/wiki/Time-of-check_to_time-of-use],
# or TOCTTOU, vulnerability that can exist when:
#
# - An ancestor directory of the entry at the target path is world writable;
# such directories include <tt>/tmp</tt>.
# - The directory tree at the target path includes:
#
# - A world-writable descendant directory.
# - A symbolic link.
#
# To avoid that vulnerability, you can use this method to remove entries:
#
# - FileUtils.remove_entry_secure: removes recursively
# if the target path points to a directory.
#
# Also available are these methods,
# each of which calls \FileUtils.remove_entry_secure:
#
# - FileUtils.rm_r with keyword argument <tt>secure: true</tt>.
# - FileUtils.rm_rf with keyword argument <tt>secure: true</tt>.
#
# Finally, this method for moving entries calls \FileUtils.remove_entry_secure
# if the source and destination are on different devices
# (which means that the "move" is really a copy and remove):
#
# - FileUtils.mv with keyword argument <tt>secure: true</tt>.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra space here.

#
# \Method \FileUtils.remove_entry_secure remove securely
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/remove/removes/

# by applying a special pre-process:
#
# - If the target path points to a directory, this method uses
# {chown(2)}[https://man7.org/linux/man-pages/man2/chown.2.html]
# and {chmod(2)}[https://man7.org/linux/man-pages/man2/chmod.2.html]
# in removing directories.
# - The owner of the target directory should be either the current process
# or the super user (root).
#
# WARNING: You must ensure that *ALL* parent directories cannot be
# moved by other untrusted users. For example, parent directories
# should not be owned by untrusted users, and should not be world
# writable except when the sticky bit set.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/set/is set/

#
# WARNING: Only the owner of the removing directory tree, or Unix super
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this paragraph can be removed, since it's mentioned in the preceding bullet point.

# user (root) should invoke this method. Otherwise this method does not
# work.
#
# For details of this security vulnerability, see Perl cases:
#
# - {CVE-2005-0448}[https://cve.mitre.org/cgi-bin/cvename.cgi?name=CAN-2005-0448].
# - {CVE-2004-0452}[https://cve.mitre.org/cgi-bin/cvename.cgi?name=CAN-2004-0452].
#
module FileUtils
VERSION = "1.6.0"

Expand Down Expand Up @@ -197,7 +252,7 @@ def remove_trailing_slash(dir) #:nodoc:
#
# Creates directories at the paths in the given +list+
# (an array of strings or a single string);
# returns +list+.
# returns +list+ if it is an array, <tt>[list]</tt> otherwise.
#
# With no keyword arguments, creates a directory at each +path+ in +list+
# by calling: <tt>Dir.mkdir(path, mode)</tt>;
Expand Down Expand Up @@ -239,7 +294,7 @@ def mkdir(list, mode: nil, noop: nil, verbose: nil)
# Creates directories at the paths in the given +list+
# (an array of strings or a single string),
# also creating ancestor directories as needed;
# returns +list+.
# returns +list+ if it is an array, <tt>[list]</tt> otherwise.
#
# With no keyword arguments, creates a directory at each +path+ in +list+,
# along with any needed ancestor directories,
Expand Down Expand Up @@ -311,7 +366,7 @@ def fu_mkdir(path, mode) #:nodoc:
#
# Removes directories at the paths in the given +list+
# (an array of strings or a single string);
# returns +list+.
# returns +list+, if it is an array, <tt>[list]</tt> otherwise.
#
# With no keyword arguments, removes the directory at each +path+ in +list+,
# by calling: <tt>Dir.rmdir(path)</tt>;
Expand Down Expand Up @@ -865,6 +920,10 @@ def copy_stream(src, dest)
# If +src+ and +dest+ are on different devices,
# first copies, then removes +src+.
#
# May cause a local vulnerability if not called with keyword argument
# <tt>secure: true</tt>;
# see {Avoiding the TOCTTOU Vulnerability}[rdoc-ref:FileUtils@Avoiding+the+TOCTTOU+Vulnerability].
#
# If +src+ is the path to a single file or directory and +dest+ does not exist,
# moves +src+ to +dest+:
#
Expand Down Expand Up @@ -898,13 +957,13 @@ def copy_stream(src, dest)
# | `-- src.txt
# `-- src1.txt
#
# - <tt>force: true</tt> - attempts to force the move;
# if the move includes removing +src+
# (that is, if +src+ and +dest+ are on different devices),
# ignores raised exceptions of StandardError and its descendants.
# Keyword arguments:
#
# - <tt>force: true</tt> - ignores raised exceptions of StandardError
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding of the previous documentation is that in this case, the :force flag only affects cases where copying and then removing the original source is needed, and in this case it would apply to the removal phase only. If this is correct, I think the previous documentation is more accurate, because it doesn't imply that raised exceptions during the copy phase are swallowed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should this say, then?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say what it says now, because I'm not sure whether your change improves things or not. Except thet I'd probably remove the first sentence "attempts to force the move", because in my mind it's quite unclear what that means. So I'd go with:

  # - <tt>force: true</tt> - if the move includes removing +src+ (that is, if +src+ and +dest+ are on different devices), ignores raised exceptions of StandardError and its descendants.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made it so.

# and its descendants.
# - <tt>noop: true</tt> - does not move files.
# - <tt>secure: true</tt> - removes +src+ securely
# by calling FileUtils.remove_entry_secure.
# - <tt>secure: true</tt> - removes +src+ securely;
# see details at FileUtils.remove_entry_secure.
# - <tt>verbose: true</tt> - prints an equivalent command:
#
# FileUtils.mv('src0', 'dest0', noop: true, verbose: true)
Expand Down Expand Up @@ -949,13 +1008,29 @@ def mv(src, dest, force: nil, noop: nil, verbose: nil, secure: nil)
alias move mv
module_function :move

# Removes entries at the paths in the given +list+
# (an array of strings or a single string);
# returns +list+, if it is an array, <tt>[list]</tt> otherwise.
#
# With no keyword arguments, removes files at the paths given in +list+:
#
# FileUtils.touch(['src0.txt', 'src0.dat'])
# FileUtils.rm(['src0.dat', 'src0.txt']) # => ["src0.dat", "src0.txt"]
#
# Keyword arguments:
#
# - <tt>force: true</tt> - ignores raised exceptions of StandardError
# and its descendants.
# - <tt>noop: true</tt> - does not remove files; returns +nil+.
# - <tt>verbose: true</tt> - prints an equivalent command:
#
# FileUtils.rm(['src0.dat', 'src0.txt'], noop: true, verbose: true)
#
# Output:
#
# Remove file(s) specified in +list+. This method cannot remove directories.
# All StandardErrors are ignored when the :force option is set.
# rm src0.dat src0.txt
#
# FileUtils.rm %w( junk.txt dust.txt )
# FileUtils.rm Dir.glob('*.so')
# FileUtils.rm 'NotExistFile', force: true # never raises exception
# FileUtils.remove is an alias for FileUtils.rm.
#
def rm(list, force: nil, noop: nil, verbose: nil)
list = fu_list(list)
Expand All @@ -971,10 +1046,13 @@ def rm(list, force: nil, noop: nil, verbose: nil)
alias remove rm
module_function :remove

# Equivalent to:
#
# Equivalent to
# FileUtils.rm(list, force: true, **kwargs)
#
# FileUtils.rm(list, force: true)
# See FileUtils.rm for keyword arguments.
#
# FileUtils.safe_unlink is an alias for FileUtils.rm_f.
#
def rm_f(list, noop: nil, verbose: nil)
rm list, force: true, noop: noop, verbose: verbose
Expand All @@ -984,24 +1062,50 @@ def rm_f(list, noop: nil, verbose: nil)
alias safe_unlink rm_f
module_function :safe_unlink

# Removes entries at the paths in the given +list+
# (an array of strings or a single string);
# returns +list+, if it is an array, <tt>[list]</tt> otherwise.
#
# remove files +list+[0] +list+[1]... If +list+[n] is a directory,
# removes its all contents recursively. This method ignores
# StandardError when :force option is set.
# May cause a local vulnerability if not called with keyword argument
# <tt>secure: true</tt>;
# see {Avoiding the TOCTTOU Vulnerability}[rdoc-ref:FileUtils@Avoiding+the+TOCTTOU+Vulnerability].
#
# FileUtils.rm_r Dir.glob('/tmp/*')
# FileUtils.rm_r 'some_dir', force: true
# For each file path, removes the file at that path:
#
# WARNING: This method causes local vulnerability
# if one of parent directories or removing directory tree are world
# writable (including /tmp, whose permission is 1777), and the current
# process has strong privilege such as Unix super user (root), and the
# system has symbolic link. For secure removing, read the documentation
# of remove_entry_secure carefully, and set :secure option to true.
# Default is <tt>secure: false</tt>.
# FileUtils.touch(['src0.txt', 'src0.dat'])
# FileUtils.rm_r(['src0.dat', 'src0.txt'])
# File.exist?('src0.txt') # => false
# File.exist?('src0.dat') # => false
#
# NOTE: This method calls remove_entry_secure if :secure option is set.
# See also remove_entry_secure.
# For each directory path, recursively removes files and directories:
#
# system('tree --charset=ascii src1')
# src1
# |-- dir0
# | |-- src0.txt
# | `-- src1.txt
# `-- dir1
# |-- src2.txt
# `-- src3.txt
# FileUtils.rm_r('src1')
# File.exist?('src1') # => false
#
# Keyword arguments:
#
# - <tt>force: true</tt> - ignores raised exceptions of StandardError
# and its descendants.
# - <tt>noop: true</tt> - does not remove entries; returns +nil+.
# - <tt>secure: true</tt> - removes +src+ securely;
# see details at FileUtils.remove_entry_secure.
# - <tt>verbose: true</tt> - prints an equivalent command:
#
# FileUtils.rm_r(['src0.dat', 'src0.txt'], noop: true, verbose: true)
# FileUtils.rm_r('src1', noop: true, verbose: true)
#
# Output:
#
# rm -r src0.dat src0.txt
# rm -r src1
#
def rm_r(list, force: nil, noop: nil, verbose: nil, secure: nil)
list = fu_list(list)
Expand All @@ -1017,13 +1121,17 @@ def rm_r(list, force: nil, noop: nil, verbose: nil, secure: nil)
end
module_function :rm_r

# Equivalent to:
#
# FileUtils.rm_r(list, force: true, **kwargs)
#
# Equivalent to
# May cause a local vulnerability if not called with keyword argument
# <tt>secure: true</tt>;
# see {Avoiding the TOCTTOU Vulnerability}[rdoc-ref:FileUtils@Avoiding+the+TOCTTOU+Vulnerability].
#
# FileUtils.rm_r(list, force: true)
# See FileUtils.rm_r for keyword arguments.
#
# WARNING: This method causes local vulnerability.
# Read the documentation of rm_r first.
# FileUtils.rmtree is an alias for FileUtils.rm_rf.
#
def rm_rf(list, noop: nil, verbose: nil, secure: nil)
rm_r list, force: true, noop: noop, verbose: verbose, secure: secure
Expand All @@ -1033,37 +1141,12 @@ def rm_rf(list, noop: nil, verbose: nil, secure: nil)
alias rmtree rm_rf
module_function :rmtree

# Securely removes the entry given by +path+,
# which should be the entry for a regular file, a symbolic link,
# or a directory.
#
# This method removes a file system entry +path+. +path+ shall be a
# regular file, a directory, or something. If +path+ is a directory,
# remove it recursively. This method is required to avoid TOCTTOU
# (time-of-check-to-time-of-use) local security vulnerability of rm_r.
# #rm_r causes security hole when:
#
# * Parent directory is world writable (including /tmp).
# * Removing directory tree includes world writable directory.
# * The system has symbolic link.
#
# To avoid this security hole, this method applies special preprocess.
# If +path+ is a directory, this method chown(2) and chmod(2) all
# removing directories. This requires the current process is the
# owner of the removing whole directory tree, or is the super user (root).
#
# WARNING: You must ensure that *ALL* parent directories cannot be
# moved by other untrusted users. For example, parent directories
# should not be owned by untrusted users, and should not be world
# writable except when the sticky bit set.
#
# WARNING: Only the owner of the removing directory tree, or Unix super
# user (root) should invoke this method. Otherwise this method does not
# work.
#
# For details of this security vulnerability, see Perl's case:
#
# * https://cve.mitre.org/cgi-bin/cvename.cgi?name=CAN-2005-0448
# * https://cve.mitre.org/cgi-bin/cvename.cgi?name=CAN-2004-0452
#
# For fileutils.rb, this vulnerability is reported in [ruby-dev:26100].
# Avoids a local vulnerability that can exist in certain circumstances;
# see {Avoiding the TOCTTOU Vulnerability}[rdoc-ref:FileUtils@Avoiding+the+TOCTTOU+Vulnerability].
#
def remove_entry_secure(path, force = false)
unless fu_have_symlink?
Expand Down