Adrian Smith
Adrian Smith

Reputation: 17553

Checking that a user-requested file is under a base directory

I have a directory hierarchy full of files on the web server, and I would like the user to be able to access any of these files by requesting them. (It's a bit more complex than this, that's why I need to access the files in the Java web application.)

The natural way to implement this would be to allow the user to specify the file, with e.g. forward slashes e.g. /my-web-endpoint?param=folder1/folder2/file.txt. I woud then find the file in Java with something like new File(baseDirectory, param). So far so good, this works.

But what if the user requests e.g. param=../something-private/file.txt or something? How should I prevent this?

  1. I could just disallow a parameter with .. in it, but is that enough? I am always a bit worried about blacklists. What if there's another syntax I hadn't thought of? (For example starting a path with a / springs to mind. But perhaps there are others.)

  2. I could use the getCanonicalFile() and then check that the path starts with the base directory, like

    File baseDirectory = ....
    String param = ...
    File desiredFile = new File(baseDirectory, param);
    if ( ! desiredFile.getCanonincalPath().startsWith(
            baseDirectory.getCanonincalPath()+File.separator))
        throw ...
    

    Is this sufficient? Can anything go wrong?

What is the correct approach?

Upvotes: 3

Views: 156

Answers (2)

gil.fernandes
gil.fernandes

Reputation: 14611

You could use a method which checks if a candidate path starts with a mandatory parent path. It seems that Java 7 nio has nice support for this.

Here is a method implementing this idea:

/**
 * Check if a candidate path starts with a mandatory parent path
 * @param mandatoryParentPath Mandatory parent path
 * @param candidate Candidate path which should be under the mandatory parent path
 * @return {@code true} if a candidate path starts with a mandatory parent path, else {@code false}
 */
/**
 * Check if a candidate path starts with a mandatory parent path
 * @param mandatoryParentPath Mandatory parent path
 * @param candidate Candidate path which should be under the mandatory parent path
 * @return {@code true} if a candidate path starts with a mandatory parent path, else {@code false}
 */
public static boolean absoluteStartsWith(Path mandatoryParentPath, Path candidate) throws IOException {
    Objects.requireNonNull(mandatoryParentPath, "Mandatory parent path should not be null");
    Objects.requireNonNull(candidate, "Candidate path should not be null");
    Path path = candidate.toRealPath();
    System.out.println(" " + candidate); // debugging code
    return path.startsWith(mandatoryParentPath.toAbsolutePath());
}

Here is a simple test to validate the method above:

public static void main (String[] args) throws java.lang.Exception
{
    Path root = Paths.get("d:/work");
    System.out.println(absoluteStartsWith(root, root.resolve("/tmp")));
    System.out.println(absoluteStartsWith(root, root.resolve("..")));
    System.out.println(absoluteStartsWith(root, root.resolve("ems")));
}

This prints out on my machine:

 d:\tmp
false
 d:\work\..
false
 d:\work\ems
true

Note: if one of the candidate paths does not exist, then an IOException will be thrown by the above absoluteStartsWith method.

Upvotes: 1

Joop Eggen
Joop Eggen

Reputation: 109567

startsWith(baseDirectory.getCanonincalPath()+"/") should be okay.

Better use the new Files & Path classes.

Path path = file.toPath();
path = path.toRealPath();
// Absolute, ".." removed, sym links resolved as no param NOFOLLOW_LINKS.
path = path.normalize(); // Removes ".."
path.startsWith(baseDir); // Base path

Upvotes: 2

Related Questions