How to prevent directory traversal when joining paths in node.js?

Question

I have a node.js webapp in which I need to concatenate two paths in a safe way. The first one (leftmost) is a constant, and the second one (rightmost) is relative to the first one and comes from untrusted user input. The resulting path should be something that is below the first path. So the situation is this:

path1 = "public/html";                // Hardcoded path.
path2 = req.query.path;               // Untrusted user input.
result = safePathJoin(path1, path2);  // Result can be e.g. public/html/index.htm,
                                      // but never private/config.xml

What I need is the function safePathJoin() that is safe against directory traversal attacks. My first naive approach is this:

safePathJoin = function(path1, path2) {
    path1 = path.normalize(path1);
    var result = path.join(path1, path2);
    return result.startsWith(path1) ? result : undefined;
}

Is this good enough? Is there a standard way to do this? Any suggestions?

Answer 1

Here's is one approach I've used in this situation:

1. path.normalize() handles all . and .., so you can be sure that if either one is present, it will be at the front of the path.
2. Remove any ../../ from the front of your path.

So:

var safeSuffix = path.normalize(unsafeSuffix).replace(/^(\.\.(\/|\\|$))+/, '');
var safeJoin = path.join(basePath, safeSuffix);

---

About your approach: checking the prefix seems like a pretty good idea to me. There are a couple of problems I see with your implementation:

• You've checked for a prefix without a trailing slash: ../html-other will resolve to public/html-other, which I guess is not what you want.
• You would run into trouble on Windows systems, where .normalize() would convert / to \, meaning no paths would work.

When I've done prefix-checking (for slightly different situations), here's what I ended up with:

function checkPrefix(prefix, candidate) {
    // .resolve() removes trailing slashes
    var absPrefix = path.resolve(prefix) + path.sep;
    var absCandidate = path.resolve(candidate) + path.sep;
    return absCandidate.substring(0, absPrefix.length) === absPrefix;
}

(Yes, I added path.sep to both, so that the prefix dir itself passes the test.)

Answer 2

Given the requirement to receive user input path that is a sub-directory of a defined root directory and allow user to access (in any way) combined root + path directory, to me the safest option would be to allow only absolute paths as an input and check that sanitized of any special directory names (., ..) input path equals the original input:

// Input

const path1 = "public/html";
const path2 = req.query.path;

// Checkout

const isNotSpecialDirName = part => !(['', '.', '..'].includes(part));

const path2Clean = path2.split(path.sep).filter(isNotSpecialDirName).join(path.sep);

// Output

if (path2Clean !== path2) {
  // Obfuscated with Not Found
  throw new Error('Not Found');
}

const result = path.join(path1, path2Clean);

Answer 3

Another option: You can just join twice.

path1 = "public/html"                // Hardcoded path.
path2 = req.query.path               // Untrusted user input.

const normalized = path.join('/', path2)
const result = path.join(path1, normalized)

In the first joining, all .., trying to go above the root folder, are ignored because the join method always resolves a parent folder of the root folder is the root folder itself.

So, for example, when path2 is ../../../test1/test2/../test3, normalized becomes /test1/test3 and result will be public/html/test1/test3.