19

Are there any standard library methods that can filter out paths which include special traversal sequences, such as ../ and all other convoluted forms of upwards directory traversal, to safeguard a file path API input from traversing upwards of a given "root" path?

I have a class that contains a root folder value member, and a member function that accepts paths to recursively delete. My goal is to make this API safe, in filtering out any input path provided to it - which would translate to a path upwards of the root folder value. The aim is that this class would be liberally used to delete files under the root path, but it would never touch anything upwards of the root path.

This is similar to the broader path traversal attack.

Methods that are too restrictive (i.e. may result in false negatives) may be fine for my specific use case, if this simplifies things, and also, my current needs are for file system paths not web ones (although, a web module for the equivalent sake might theoretically work here).

matanster
  • 15,072
  • 19
  • 88
  • 167
  • How are you exposing those paths to the web? In general, frameworks such as Spring do a reasonably good job of mitigating against those attacks, so you would seldom have to add any other layer of security over it. – Makoto Oct 12 '15 at 14:20
  • I'm not. This is for a backend devops related API, which includes some file deletion logic, and it needs to avoid deleting files outside a given root folder, without the help of OS file system permissions. – matanster Oct 12 '15 at 14:21
  • What kind of paths are we talking about here? `java.nio.file.Path` has methods to consolidate relative file paths onto a base for example. – biziclop Oct 12 '15 at 14:22
  • There are still some gaps to how this could be an attack. I can see how it can be a *concern*, but if it were an attack, then you'd have to have not properly implemented the path traversal in the first place. Give us an example of what a DELETE request would look like. – Makoto Oct 12 '15 at 14:22
  • A delete API call (not dealing with web requests here) would just provide a path parameter. Such methods as you mention might be helpful indeed. – matanster Oct 12 '15 at 14:24
  • So basically you've got a tool that makes calls down to the system, and you want to be sure that it doesn't walk *too* far? This is still not explaining enough of what the actual code is doing. – Makoto Oct 12 '15 at 14:30
  • 3
    By the way - I've removed the request for libraries, since that's a big reason why others are trying to close the question. It has potential, but adding the libraries bit makes it a huge close target. – Makoto Oct 12 '15 at 14:31
  • @Makoto Thanks for the edit. I improved the question along your comment now, it was definitely called for! – matanster Oct 12 '15 at 14:42

2 Answers2

35

You can use Path.normalize() to strip out ".." elements (and their preceding elements) from a path — e.g. it'll turn "a/b/../c" into "a/c". Note that it won't strip out a ".." at the beginning of a path, since there's no preceding directory component for it to remove as well. So if you're going to prepend another path, do that first, then normalize the result.

You can also use Path.startsWith(Path) to check whether one path is a descendant of another. And Path.isAbsolute() tells you, unsurprisingly, whether a path is absolute or relative.

Here's how I'd process the untrusted paths coming into the API:

/**
 * Resolves an untrusted user-specified path against the API's base directory.
 * Paths that try to escape the base directory are rejected.
 *
 * @param baseDirPath  the absolute path of the base directory that all
                     user-specified paths should be within
 * @param userPath  the untrusted path provided by the API user, expected to be
                  relative to {@code baseDirPath}
 */
public Path resolvePath(final Path baseDirPath, final Path userPath) {
  if (!baseDirPath.isAbsolute()) {
    throw new IllegalArgumentException("Base path must be absolute");
  }

  if (userPath.isAbsolute()) {
    throw new IllegalArgumentException("User path must be relative");
  }

  // Join the two paths together, then normalize so that any ".." elements
  // in the userPath can remove parts of baseDirPath.
  // (e.g. "/foo/bar/baz" + "../attack" -> "/foo/bar/attack")
  final Path resolvedPath = baseDirPath.resolve(userPath).normalize();

  // Make sure the resulting path is still within the required directory.
  // (In the example above, "/foo/bar/attack" is not.)
  if (!resolvedPath.startsWith(baseDirPath)) {
    throw new IllegalArgumentException("User path escapes the base path");
  }

  return resolvedPath;
}
newbie
  • 24,286
  • 80
  • 201
  • 301
Wyzard
  • 33,849
  • 3
  • 67
  • 87
3

You do not need to use a third-party library to do this. The file APIs that Java provides give you the ability to verify that a file is a descendent of another.

Path.resolve(String) will resolve parent directory references, absolute, and relative paths. If an absolute path is passed as an argument to the resolve method it returns the absolute path. It does not guarantee that the returned value is a descendent of the path the method was invoked on.

You can check that a path is a descendent of another path by using the Path.startsWith(Path) method.

Path root = java.nio.file.Files.createTempDirectory(null);
Path relative = root.resolve(pathAsString).normalize();
if (!relative.startsWith(root)) {
    throw new IllegalArgumentException("Path contains invalid characters");
}

When pathAsString contains references to parent directories or was an absolute path, relative can reference a file that is not contained in root. When this is the case you can throw an exception before any modifications to the file are permitted.

Aaron
  • 574
  • 3
  • 8
  • 1
    So why does that work? **Explain yourself.** – Makoto Oct 12 '15 at 14:58
  • 1
    Warning: Do not do this! This method is worse than nothing, because it gives a false sense of security. You can perform directory traversal attacks with this check in place by simply ... *traversing*! For example: `pathAsString = "..";`. – Zero3 Dec 15 '15 at 16:59
  • 2
    The relative path should get normalized before the comparison. I updated the answer to reflect the correction. – Aaron Dec 15 '15 at 18:25