Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8262822

Provide Path.resolve for safely resolving children

XMLWordPrintable

    • generic
    • generic

      A DESCRIPTION OF THE PROBLEM :
      Despite being discouraged from a security standpoint, often applications use file paths from external untrusted sources, e.g. to interact with legacy systems.
      This had lead to exploits like Directory Traversal or Zip Slip. One of the issues why so many applications are vulnerable to these exploits is that the JDK java.io.File and java.nio.file.Path classes are implemented in a way which makes their usage convenient for the developer: Get a path from a (trusted) user and resolve it against a different path, handling different roots, abstract vs. relative and so on.
      Obviously this does not work very well when the path comes from an untrusted user.

      Implementing path resolution in a safe way is currently rather difficult due to the JDK not offering such methods directly, instead complex constructs such as the one proposed by Snyk (finders of the ZipSlip vulnerability) have to be used: https://snyk.io/research/zip-slip-vulnerability#java

      This request for enhancement therefore requests the following methods for java.nio.file.Path:
      - Path.resolveChild(String other)
      - Path.resolveChild(Path other)
      - Path.resolveDirectChild(String other)
      - Path.resolveDirectChild(Path other)

      These methods and their default implementation are described below (for simplicity in an interface Path2 extending java.nio.file.Path to avoid the overhead of editing JDK sources; though the actual implementation should be in java.nio.file.Path):

      public interface Path2 extends Path {
          /**
           * Converts the given path string to a {@code Path} and resolves it against this {@code Path},
           * throwing an exception if the resulting path is not a child of this path.
           * The path is resolved in exactly the manner specified by the
           * {@link #resolveChild(Path)} method.
           *
           * @param child
           * the path to resolve against this path
           * @return
           * the resulting path
           * @throws InvalidPathException
           * if the path string cannot be converted to a Path
           * @throws IllegalArgumentException
           * if the other path does not meet any of the requirements for being a child path
           * @see {@link #resolveDirectChild(String)}
           */
          default Path resolveChild(String child) throws InvalidPathException, IllegalArgumentException {
              return resolveChild(getFileSystem().getPath(child));
          }
          
          /**
           * Resolves the given path against this {@code Path}, throwing an exception if the resulting
           * path is not a child of this path. The path is resolved in exactly the manner specified by
           * the {@link #resolve(Path) resolve} method. Afterwards it is verified that the result is a
           * child of this path. The following requirements have to be met, otherwise an
           * {@link IllegalArgumentException} is thrown:
           * <ul>
           * <li>the other path must not be {@link #isAbsolute() absolute}
           * <li>the other path must not have a {@link #getRoot() root}
           * <li>the other path must not contain any name elements which allow
           * navigating the element hierarchy;
           * <br>the precise definition of this is implementation dependent, but for example
           * "{@code .}" and "{@code ..}", indicating the current and parent directory for some
           * file systems, will not be allowed
           * <li>the result path must be a true child (or grandchild, ...) of this path, it must
           * not be equal to this path
           * </ul>
           *
           * @apiNote
           * This method is intended for cases where a path from an untrusted source has to
           * be resolved. Note however that it is in general <em>not recommended</em> to use
           * untrusted file paths for file system access. This method might not detect reserved
           * file names or too long file names.
           *
           * @implSpec
           * The default implementation of this method performs detection of name elements
           * allowing element hierarchy navigation through usage of {@link #normalize()}.
           * Subtypes should override this method if they can provide a better implementation.
           *
           * @param child
           * the path to resolve against this path
           * @return
           * the resulting path
           * @throws IllegalArgumentException
           * if the other path does not meet any of the requirements for being a child
           * path
           * @see #resolveDirectChild(Path)
           */
          default Path resolveChild(Path child) throws IllegalArgumentException {
              /*
               * Don't permit any root:
               * - If different root, result would not be child of this
               * -> have to throw exception
               * - If same root, would allow an adversary to know that their
               * provided root was guessed 'correctly' because no
               * exception is thrown
               */
              if (child.getRoot() != null) {
                  throw new IllegalArgumentException("Child path has root");
              }
              // Don't permit absolute because when resolved against this, would
              // discard path of this
              else if (child.isAbsolute()) {
                  throw new IllegalArgumentException("Child path is absolute");
              }
              
              /*
               * Resolve path against dummy to detect `.` or `..`;
               * cannot resolve against `this` because if this is empty path,
               * `this.resolve(other).normalize()` would not get rid of leading `..`
               *
               * Additionally don't allow any `.` or `..` at all, even if they
               * represent a child path after resolution, e.g. "a/b".resolve("../b/c")
               * Because even the fact that the result is valid gives an adversary
               * information they should not have; e.g. here they would know that
               * the parent is `b` because for resolve("../x/c") an exception
               * would have been thrown
               */
              // TODO: Maybe this should be relaxed to allow `.` and `..` as long
              // as they only affect the to-be-resolved child but not the
              // parent; could be implemented by only checking that
              // otherDummyNormalized starts with dummy followed by first
              // name element of `child.normalize()` (if child has no name
              // elements it is not allowed either because it is not a true
              // child)
              Path dummy = getFileSystem().getPath("dummy");
              Path otherDummyNormalized = dummy.resolve(child).normalize();
              // Check if `normalize()` removed any elements
              if (otherDummyNormalized.getNameCount() != 1 + child.getNameCount()) {
                  throw new IllegalArgumentException("Invalid child path");
              }
              // Verify that normalization did not change any name elements
              if (!otherDummyNormalized.startsWith(dummy) || !otherDummyNormalized.endsWith(child)) {
                  throw new IllegalArgumentException("Invalid child path");
              }
              
              Path result = resolve(child);
              
              Path resultNormalized = result.normalize();
              Path thisNormalized = normalize();
              int minDiff;
              if (isEmptyPath(thisNormalized)) {
                  minDiff = 0;
                  // Detect case "".resolve("")
                  if (isEmptyPath(resultNormalized)) {
                      throw new IllegalArgumentException("Invalid child path");
                  }
              }
              // Only perform further checks when `this` is not empty path "" because for "".resolve(other)
              // startsWith(...) will be false
              else {
                  minDiff = 1;
                  // Sanity check; probably already covered by normalization checks above
                  if (!resultNormalized.startsWith(thisNormalized)) {
                      throw new IllegalArgumentException("Invalid child path");
                  }
              }
              
              // Verify that result is actually a 'true' child
              if (resultNormalized.getNameCount() - thisNormalized.getNameCount() < minDiff) {
                  throw new IllegalArgumentException("Invalid child path");
              }
              
              return result;
          }
          
          /**
           * Returns if {@code p} is "".
           */
          private static boolean isEmptyPath(Path p) {
              return p.getRoot() == null && p.getNameCount() == 1 && p.getName(0).toString().isEmpty();
          }
          
          /**
           * Converts the given path string to a {@code Path} and resolves it against this {@code Path},
           * throwing an exception if the resulting path is not a direct child of this path.
           * The path is resolved in exactly the manner specified by the
           * {@link #resolveDirectChild(Path)} method.
           *
           * @param child
           * the path to resolve against this path
           * @return
           * the resulting path
           * @throws InvalidPathException
           * if the path string cannot be converted to a Path
           * @throws IllegalArgumentException
           * if the other path does not meet any of the requirements for being a direct child
           * path
           * @see {@link #resolveChild(String)}
           */
          default Path resolveDirectChild(String child) throws InvalidPathException, IllegalArgumentException {
              return resolveDirectChild(getFileSystem().getPath(child));
          }
          
          /**
           * Resolves the given path against this {@code Path}, throwing an exception if the resulting
           * path is not a direct child of this path. The path is resolved in exactly the manner specified
           * by the {@link #resolve(Path) resolve} method. Afterwards it is verified that the result is a
           * direct child of this path. The following requirements have to be met, otherwise an
           * {@link IllegalArgumentException} is thrown:
           * <ul>
           * <li>the other path must not be {@link #isAbsolute() absolute}
           * <li>the other path must not have a {@link #getRoot() root}
           * <li>the other path must not contain any name elements which allow
           * navigating the element hierarchy;
           * <br>the precise definition of this is implementation dependent, but for example
           * "{@code .}" and "{@code ..}", indicating the current and parent directory for some
           * file systems, will not be allowed
           * <li>the result path must be a direct child, i.e. its {@link #getNameCount() name count}
           * must be equal to {@code name count + 1} of this (except for the case where {@code this}
           * is an empty path, in which case name counts must be equal and the {@code child} must
           * not be empty itself)
           * </ul>
           *
           * @apiNote
           * This method is intended for cases where a path from an untrusted source has to
           * be resolved. Note however that it is in general <em>not recommended</em> to use
           * untrusted file paths for file system access. This method might not detect reserved
           * file names or too long file names.
           *
           * @implSpec
           * The default implementation delegates to {@link #resolveChild(Path)} and then
           * verifies that the result is a direct child.
           *
           * @param child
           * the path to resolve against this path
           * @return
           * the resulting path
           * @throws IllegalArgumentException
           * if the other path does not meet any of the requirements for being a direct child
           * path
           * @see #resolveChild(Path)
           */
          default Path resolveDirectChild(Path child) throws IllegalArgumentException {
              Path result = resolveChild(child);
              if (result.getNameCount() - 1 > getNameCount()) {
                  throw new IllegalArgumentException("Not a direct child");
              }
              return result;
          }
          
          /*
           * INTERNAL TESTING METHODS
           */
          
          private static Path2 path(String path) {
              Path delegate = Path.of(path);
              return new Path2() {
                  @Override
                  public FileSystem getFileSystem() {
                      return delegate.getFileSystem();
                  }

                  @Override
                  public boolean isAbsolute() {
                      return delegate.isAbsolute();
                  }

                  @Override
                  public Path getRoot() {
                      return delegate.getRoot();
                  }

                  @Override
                  public Path getFileName() {
                      return delegate.getFileName();
                  }

                  @Override
                  public Path getParent() {
                      return delegate.getParent();
                  }

                  @Override
                  public int getNameCount() {
                      return delegate.getNameCount();
                  }

                  @Override
                  public Path getName(int index) {
                      return delegate.getName(index);
                  }

                  @Override
                  public Path subpath(int beginIndex, int endIndex) {
                      return delegate.subpath(beginIndex, endIndex);
                  }

                  @Override
                  public boolean startsWith(Path other) {
                      return delegate.startsWith(other);
                  }

                  @Override
                  public boolean endsWith(Path other) {
                      return delegate.endsWith(other);
                  }

                  @Override
                  public Path normalize() {
                      return delegate.normalize();
                  }

                  @Override
                  public Path resolve(Path other) {
                      return delegate.resolve(other);
                  }

                  @Override
                  public Path relativize(Path other) {
                      return delegate.relativize(other);
                  }

                  @Override
                  public URI toUri() {
                      return delegate.toUri();
                  }

                  @Override
                  public Path toAbsolutePath() {
                      return delegate.toAbsolutePath();
                  }

                  @Override
                  public Path toRealPath(LinkOption... options) throws IOException {
                      return delegate.toRealPath(options);
                  }

                  @Override
                  public WatchKey register(WatchService watcher, Kind<?>[] events, Modifier... modifiers) throws IOException {
                      return delegate.register(watcher, events, modifiers);
                  }

                  @Override
                  public int compareTo(Path other) {
                      return delegate.compareTo(other);
                  }

                  @Override
                  public boolean equals(Object other) {
                      return delegate.equals(other);
                  }

                  @Override
                  public int hashCode() {
                      return delegate.hashCode();
                  }

                  @Override
                  public String toString() {
                      return delegate.toString();
                  }
              };
          }
          
          private static void assertThrows(Runnable r, String expectedMessage) {
              try {
                  r.run();
                  throw new AssertionError("Expected exception");
              } catch (IllegalArgumentException e) {
                  String message = e.getMessage();
                  if (!expectedMessage.equals(message)) {
                      throw new AssertionError("Expected exception message: " + expectedMessage + "\n got: " + message);
                  }
              }
          }
          
          static void main(String[] args) {
              Map.of(
                  path("").resolveChild("a"), "a",
                  path("").resolveChild("a/b"), "a/b",
                  path("C:").resolveChild("a"), "C:a",
                  path("C:/").resolveChild("a/b"), "C:/a/b",
                  path("C:/").resolveDirectChild("a"), "C:/a",
                  path("C:/").resolveDirectChild("b/"), "C:/b",
                  path("").resolveDirectChild("b"), "b"
              )
                  .forEach((actual, expected) -> {
                      Path2 expectedPath = path(expected);
                      if (!expectedPath.equals(actual)) {
                          throw new AssertionError("Expected: " + expected + "\n got: " + actual);
                      }
                  });
              
              assertThrows(() -> path("").resolveChild(".."), "Invalid child path");
              assertThrows(() -> path("").resolveChild("."), "Invalid child path");
              assertThrows(() -> path("").resolveChild(""), "Invalid child path");
              assertThrows(() -> path("C:").resolveChild("C:a"), "Child path has root");
              assertThrows(() -> path("C:").resolveChild("C:/a"), "Child path has root");
              assertThrows(() -> path("a/b").resolveChild("/a/c"), "Child path has root");
              assertThrows(() -> path("/a/b").resolveChild("/a/b/c"), "Child path has root");
              assertThrows(() -> path("a/b").resolveChild("../b/c"), "Invalid child path");
              assertThrows(() -> path("a/b").resolveChild("a/b/../c/../d"), "Invalid child path");
              assertThrows(() -> path("a/b").resolveDirectChild(""), "Invalid child path");
              assertThrows(() -> path("a/b").resolveDirectChild("a/b"), "Not a direct child");
              assertThrows(() -> path("a/b").resolveDirectChild("a/b/.."), "Invalid child path");
              assertThrows(() -> path("").resolveDirectChild(""), "Invalid child path");
              assertThrows(() -> path("C:a").resolveDirectChild("C:a"), "Child path has root");
              assertThrows(() -> path("C:a").resolveDirectChild("C:/a"), "Child path has root");
              assertThrows(() -> path("C:a").resolveDirectChild("/a"), "Child path has root");
              assertThrows(() -> path("a").resolveDirectChild("/a"), "Child path has root");
          }
      }


            bpb Brian Burkhalter
            webbuggrp Webbug Group
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated: