[API Proposal]: Add a [System.IO.Path] method to ensure that a relative path stays inside a directory

[MatejKafka]

Background and motivation

See golang/go#56219 for a similar feature request for Go.

It is common for programs to accept filenames from untrusted sources. For example, an archive extractor might create files based on names in the archive, or a web server may serve the content of local files identified by a URL path. In both cases, the untrusted path is a relative path that should never leave a known directory specified in the program, e.g. if serving files from D:\webserver, paths like ../dir must not allow the user to read D:\dir.

Currently, user has to do this validation by hand, either by trying to parse the relative path directly (which is non-trivial and ripe with edge cases), or doing something like

var resolvedPath = Path.GetFullPath(Path.Combine(basePath, untrustedPath));
if (resolvedPath.StartsWith(basePath + Path.DirectorySeparatorChar)) { ... }

, of which I've also seen multiple incorrect variants on StackOverflow and similar sites (typically forgetting to add the directory separator to basePath). For either of these options, I'm not convinced that I can write a correct implementation without a lot of fuzzing.

API Proposal

Add a new method to System.IO.Path, bool IsPathLocal(string relativePath), which returns true if relativePath does not leave the current directory. The method should not be dependent on any particular directory, operating only on the string path, without referencing the filesystem.

API Usage

var untrustedPath = readClientRequest();
if (!Path.IsPathLocal(untrustedPath)) {
  sendError("can't touch that");
  return;
}
// ...
sendFileToClient(Path.Join(basePath, untrustedPath));

Alternative Designs

.. handling

I see two possibilities of how paths containing .. could be handled:

1. Allow the path, as long as remains inside the directory (more complicated).
2. Reject any path that contains .. as a segment. This is more restrictive, but I'd assume that in practice, it would work as well as the first variant for common scenarios while being much simpler to implement.

Retrieving the full path

Alternatively, the API could be implemented as a method string? JoinLocal(string basePath, string relativePath) (not sure about the name), which also receives the base directory and returns a string? which either contains the resolved absolute path, or is null if relativePath is not local. This might be more performant, since the API user will typically want to eventually resolve the path, but combines validation and resolution into a single API, which might be less flexible in cases where the API user wants to do the validation upfront, but only resolve the path later in the program.

Risks

No response

[ghost]

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

See golang/go#56219 for a similar feature request for Go.

It is common for programs to accept filenames from untrusted sources. For example, an archive extractor might create files based on names in the archive, or a web server may serve the content of local files identified by a URL path. In both cases, the untrusted path is a relative path that should never leave a known directory specified in the program, e.g. if serving files from D:\webserver, paths like ../dir must not allow the user to read D:\dir.

Currently, user has to do this validation by hand, either by trying to parse the relative path directly (which is non-trivial and ripe with edge cases), or doing something like

var resolvedPath = Path.GetFullPath(Path.Combine(basePath, untrustedPath));
if (resolvedPath.StartsWith(basePath + Path.DirectorySeparatorChar)) { ... }

, of which I've also seen multiple incorrect variants on StackOverflow and similar sites (typically forgetting to add the directory separator to basePath). For either of these options, I'm not convinced that I can write a correct implementation without a lot of fuzzing.

API Proposal

Add a new method to System.IO.Path, bool IsPathLocal(string relativePath), which returns true if relativePath does not leave the current directory. The method should not be dependent on any particular directory, operating only on the string path, without referencing the filesystem.

API Usage

var untrustedPath = readClientRequest();
if (!Path.IsPathLocal(untrustedPath)) {
  sendError("can't touch that");
  return;
}
// use `untrustedPath`

Alternative Designs

.. handling

I see two possibilities of how paths containing .. could be handled:

1. Allow the path, as long as remains inside the directory (more complicated).
2. Reject any path that contains .. as a segment. This is more restrictive, but I'd assume that in practice, it would work as well as the first variant for common scenarios while being much simpler to implement.

Retrieving the full path

Alternatively, the API could be implemented as a method string? CombineLocal(string basePath, string relativePath) (not sure about the name), which also receives the base directory and returns a string? which either contains the resolved absolute path, or is null if relativePath is not local. This might be more performant, since the API user will typically want to eventually resolve the path, but combines validation and resolution into a single API, which might be less flexible in cases where the API user wants to do the validation upfront, but only resolve the path later in the program.

Risks

No response

Author:	MatejKafka
Assignees:	-
Labels:	api-suggestion, area-System.IO, untriaged
Milestone:	-

[Tornhoof]

Before you try working with startswith, take a look at https://learn.microsoft.com/en-us/dotnet/api/system.io.path.getrelativepath?view=net-7.0#system-io-path-getrelativepath(system-string-system-string)

and then check if the return of that method contains .. or equals the second arg.

That method also takes care of Path.GetFullPath for you.

[Clockwork-Muse]

From a usefulness standpoint, something that only checks if a path is a subdirectory of working directory seems overly restrictive. And potentially its own security hole, because the working directory might not be what you expect, anyways (as opposed to the source directory of the application).

I personally would prefer some sort of IsSubdirectoryOf(...)-type method (although resolution in some edge cases might be fun), if we're going that route.

[danmoseley]

I'm not sure that any examination of strings can provide a bulletproof guarantee, given variations in OS and file system behaviors, hard links, and so forth. This is something code access security (CAS) tried to do. The only sure way is likely to have the OS tell you, somehow.
Cc @JeremyKuhne for thoughts

[MatejKafka]

@Clockwork-Muse

only checks if a path is a subdirectory of working directory

The proposed API is not dependent on the current working directory. If the IsPathLocal method returns true, it should be safe to combine the path with any base path you want (Path.Join(basePath, untrustedPath)).

---

@danmoseley I'm also not sure about that, but as long as symlinks are not involved, I haven't found any path where interaction with the filesystem is necessary to determine whether it's safe. If such paths exist, it might be necessary to use a different implementation for each platform (which is consistent with what other methods in System.IO.Path do).

[JeremyKuhne]

This is a dupe of #87581 I think. I gave some commentary there on design of such an API: #87581 (comment)