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.
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
proposal: os: safer file open functions #67002
Comments
is this essentially https://pkg.go.dev/github.com/google/safeopen with Beneath -> In ? that also has a ReadFile / WriteFile variant which I'd use more then the create version. |
The design of this proposal is influenced by github.com/google/safeopen, but differs in a few areas. (Sorry, I really should have mentioned safeopen as prior art.) Of the three parts of this proposal:
|
Yes, please. When I was working on safe file operations and it turned out to be hard to do correctly without OS support. Without |
What, if any, changes would be made to "io/fs"? Ideally, there is a mirror of these APIs in that package. |
If we wanted to extend this proposal to package fs
// An OpenFile is a directory file whose entries may be opened with the Open method.
type OpenFile interface {
File
// Open opens the named file in the directory.
//
// When Open returns an error, it should be of type *PathError
// with the Op field set to "openat", the Path field set to name,
// and the Err field describing the problem.
//
// Open should reject attempts to open names that do not
// satisfy ValidPath(name), returning a *PathError with Err set to
// ErrInvalid or ErrNotExist.
Open(name string) (File, error)
} A more interesting question is I don't think we can change The interaction between Perhaps we should add a version of |
Perhaps the new names should include an At suffix to make clear to casual readers of the API that these are not the usual open system calls. Either way, the three new methods should probably reference some shared section of documentation on the concept of the |
I presume you mean the new method names? The functions have an "In" suffix. We could also include the In suffix on the methods; I waffled on whether it belongs there or not:
I'm trying to avoid the suffix "At" to make it clear that none of these calls are precisely |
Fair enough. Should the |
Probably, for consistency. |
Directory traversal vulnerabilities are a common class of vulnerability, in which an attacker tricks a program into opening a file that it did not intend. These attacks often take the form of providing a relative pathname such as
"../../../etc/passwd"
, which results in access outside an intended location. CVE-2024-3400 is a recent, real-world example of directory traversal leading to an actively exploited remote code execution vulnerability.A related, but less commonly exploited, class of vulnerability involves unintended symlink traversal, in which an attacker creates a symbolic link in the filesystem and manipulates the target into following it.
I propose adding several new functions to the os package to aid in safely opening files with untrusted filename components and defending against symlink traversal.
It is very common for programs to open a file in a known location using an untrusted filename. Programs can avoid directory traversal attacks by first validating the filename with a function like
filepath.IsLocal
. Defending against symlink traversal is harder.I propose adding functions to open a file in a location:
The
OpenFileIn
,OpenIn
, andCreateIn
family of functions safely open a file within a given location, defending against directory traversal, symlinks to unexpected locations, and unexpected access to Windows device files.All modern Unix systems that I know of provide an
openat
call, to open a file relative to an existing directory handle (FD). Windows provides an equivalent (NtCreateFile
withObjectAttributes
including aRootDirectory
). Of the supported Go ports, I believe only js and plan9 do not supportopenat
or an equivalent.I propose adding support for
openat
-like behavior toos.File
:Like the top-level
CreateIn
,OpenIn
, andOpenFileIn
, the methods defend against accessing files outside the given directory. This is unlike the default behavior ofopenat
, which permits absolute paths, relative paths outside the root, and symlink traversal outside the root. (It corresponds to Linux'sopenat2
with theRESOLVE_BENEATH
flag.)A property of
openat
is that it follows a file across renames: If you open a directory, rename the directory, and useopenat
on the still-open FD, access is relative to the directory's new location. We cannot support this behavior on platforms which don't haveopenat
or an equivalent (plan9 and js). We could fall back to operating purely on filenames, such thatf.OpenIn(x)
is equivalent toos.OpenIn(f.Name(), x)
. However, this seems potentially hazardous. I propose, therefore, thatFile.CreateIn
,File.OpenIn
, andFile.OpenFileIn
return anerrors.ErrUnsupported
error on these platforms.The above functions defend against symlink traversal that leads outside of the designated root directory. Some users may wish to defend against symlink traversal entirely. Many modern operating systems provide an easy way to disable symlink following: Linux has
RESOLVE_NO_SYMLINKS
, Darwin hasO_NOFOLLOW_ANY
, and some other platforms have equivalents.I propose adding support for disabling symlink traversal to the
os
package:O_NOFOLLOW_ANY
may be passed toOpenFile
,OpenFIleIn
, orFile.OpenFIle
to disable symlink traversal in any component of the file name. ForOpenFileIn
, symlinks would still be permitted in the directory component.On platforms which do not support the equivalent of
O_NOFOLLOW_ANY
/RESOLVE_NO_SYMLINKS
natively, theos
package will use successiveopenat
calls withO_NOFOLLOW
to emulate it. On platforms with noopenat
(plan9 and js), open operations will return an error whenO_NOFOLLOW_ANY
is specified.The text was updated successfully, but these errors were encountered: