os: safer file open functions

@gabyhelp

Please see the updated proposal in #67002 (comment)

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:

package os

// OpenFileIn opens the named file in the named directory.
//
// If the file contains relative path components (..), no component may
// refer to a location outside the parent directory. The file may not be
// "", an absolute path, or (on Windows) a reserved device name such as "NUL".
// The file may refer to the directory itself (.).
//
// If any component of the named file references a symbolic link
// referencing a location out of the parent directory,
// OpenFileIn returns an error.
//
// OpenFileIn otherwise behaves like OpenFile.
func OpenFileIn(parent, name string, flag int, perm FileMode) (*File, error)

// CreateIn creates or truncates the named file in the named parent directory.
// It applies the same constraints on files as [OpenFileIn].
// It otherwise behaves like [Create].
func CreateIn(parent, name string) (*File, error)

// Open opens the named file in the named parent directory for reading.
// It applies the same constraints on files as [OpenFileIn].
// It otherwise behaves like [Open].
func OpenIn(parent, name string) (*File, error)

The OpenFileIn, OpenIn, and CreateIn 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 with ObjectAttributes including a RootDirectory). Of the supported Go ports, I believe only js and plan9 do not support openat or an equivalent.

I propose adding support for openat-like behavior to os.File:

package os

// OpenFile opens the named file in the directory associated with the file f.
//
// If the file contains relative path components (..), no component may
// refer to a location outside the parent directory. The file may not be
// "", an absolute path, or (on Windows) a reserved device name such as "NUL".
//
// If any component of the named file references a symbolic link
// referencing a location out of the parent directory,
// OpenFile returns an error.
func (f *File) OpenFile(name string, flag int, perm FileMode) (*File, error)

// Create creates or truncates the named file in
// the directory associated with the file f.
// It applies the same constraints on files as [File.OpenFile].
func (f *File) Create(name string) (*File, error)

// Open opens the named file in the directory associated with the file f for reading.
// It applies the same constraints on files as [File.OpenFile].
func (f *File) OpenIn(name string) (*File, error)

Like the top-level CreateIn, OpenIn, and OpenFileIn, the methods defend against accessing files outside the given directory. This is unlike the default behavior of openat, which permits absolute paths, relative paths outside the root, and symlink traversal outside the root. (It corresponds to Linux's openat2 with the RESOLVE_BENEATH flag.)

A property of openat is that it follows a file across renames: If you open a directory, rename the directory, and use openat on the still-open FD, access is relative to the directory's new location. We cannot support this behavior on platforms which don't have openat or an equivalent (plan9 and js). We could fall back to operating purely on filenames, such that f.OpenIn(x) is equivalent to os.OpenIn(f.Name(), x). However, this seems potentially hazardous. I propose, therefore, that File.CreateIn, File.OpenIn, and File.OpenFileIn return an errors.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 has O_NOFOLLOW_ANY, and some other platforms have equivalents.

I propose adding support for disabling symlink traversal to the os package:

const (
	// O_NOFOLLOW_ANY, when included in the flags passed to [OpenFile], [OpenFileIn],
	// or [File.OpenFile], disallows resolution of symbolic links anywhere in the
	// named file.
	//
	// O_NOFOLLOW_ANY affects the handling of symbolic links in all components
	// of the filename. (In contrast, the O_NOFOLLOW flag supported by many
	// platforms only affects resolution of the last path component.) 
	//
	// O_NOFOLLOW_ANY does not disallow symbolic links in the parent directory name
	// parameter of [OpenFileIn].
	//
	// O_NOFOLLOW_ANY does not affect traversal of hard links, Windows junctions,
	// or Plan 9 bind mounts.
	//
	// On platforms which support symbolic links but do not provide a way to
	// disable symbolic link traversal (GOOS=js), open functions return an error
	// if O_NOFOLLOW_ANY is provided.
	O_NOFOLLOW_ANY int = (some value)
)

O_NOFOLLOW_ANY may be passed to OpenFile, OpenFIleIn, or File.OpenFIle to disable symlink traversal in any component of the file name. For OpenFileIn, 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, the os package will use successive openat calls with O_NOFOLLOW to emulate it. On platforms with no openat (plan9 and js), open operations will return an error when O_NOFOLLOW_ANY is specified.

@gabyhelp's overview of this issue: #67002 (comment)

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:

os.OpenIn is essentially safeopen.OpenBeneath.
File.Open is a slightly more limited but safer version of openat, and has no equivalent in safeopen.
O_NOFOLLOW_ANY has no equivalent in safeopen.

ReadFileIn and WriteFileIn seem like a useful and logical extension 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 O_NOFOLLOW, you have to slowly check every segment for symlinks before traversing into it. For the naive implementation, how do you protect against TOCTOU bugs? At the moment that you check some path segment and verify that it's not a symlink (or a safe one) and then proceed to descend into it, some other process (or goroutine) could have asynchronously changed the target.

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 io.FS, I believe the one addition would be:

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 os.DirFS. Currently, DirFS has two documented limitations: It follows symlinks out of the directory tree, and if the FS root is a relative path then it will be affected by later Chdir calls.

I don't think we can change DirFS's symlink-following behavior: It's documented, and it's a behavior that a user could reasonably depend on.

The interaction between DirFS and Chdir seems less likely to be something a user would depend on, but it is documented. I'm not sure if we can change it at this point, but perhaps.

Perhaps we should add a version of DirFS that opens the directory root at creation time (retaining a handle to it even if the current working directory changes or the root is renamed), and refuses to follow symlinks out of the root. I'm not sure if that should be part of this proposal or a separate one.

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 at-suffixed operations.

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:

func (f *File) OpenFileIn(name string, flag int, perm FileMode) (*File, error)
func (f *File) CreateIn(name string) (*File, error)
func (f *File) OpenIn(name string) (*File, error)

I'm trying to avoid the suffix "At" to make it clear that none of these calls are precisely openat. openat(2) permits escaping from the root directory via absolute or relative paths, and doesn't do anything about symlink traversal. (Linux has openat2(2), which is quite configurable. The proposed *In functions are essentially openat2 with the RESOLVE_BENEATH flag.)

Fair enough. Should the fs.OpenFile.Open method also be named OpenIn?

Should the fs.OpenFile.Open method also be named OpenIn?

Probably, for consistency.

Are we missing RemoveIn?

We should probably have RemoveIn as well:

// RemoveIn removes the named file or (empty) directory.
// It applies the same constraints on files as [OpenFileIn].
// It otherwise behaves like [Remove].
func RemoveIn(parent, name string) error

// Remove removes the named file or (empty) directory
// in the directory associated with the file f.
// It applies the same constraints on files as [File.OpenFile].
func (f *File) RemoveIn(name string) error

Perhaps also RemoveAllIn?

Will there be an equivalent of Walk/WalkDir for os.Root?

Will there be an equivalent of Walk/WalkDir for os.Root?

Root.FS gives you an fs.FS which you can use with io/fs.WalkDir.

Change https://go.dev/cl/674116 mentions this issue: os: add Root.MkdirAll

os: add Root.RemoveAll

os: add Root.MkdirAll

Proposal fully implemented, except for Root.Truncate which I don't intend to add. (Rationale: #67002 (comment))

os: add Root.RemoveAll

os: add Root.MkdirAll

check for dir/symlink conflicts on checkout/pull

Navigation Menu

Repository navigation

os: safer file open functions #67002

142 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

os: safer file open functions #67002

Description

Activity

seankhliao commented on Apr 23, 2024

neild commented on Apr 23, 2024

dsnet commented on Apr 24, 2024

dsnet commented on Apr 24, 2024

neild commented on Apr 24, 2024

adonovan commented on Apr 26, 2024

neild commented on Apr 26, 2024

adonovan commented on Apr 26, 2024

neild commented on Apr 26, 2024

rsc commented on May 29, 2024

neild commented on May 29, 2024

142 remaining items

cedws commented on May 5, 2025

neild commented on May 6, 2025

gopherbot commented on May 19, 2025

neild commented on May 23, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions