security vulnerability with directory #771

Closed
mikerobe opened this Issue Apr 1, 2013 · 1 comment

Comments

Projects
None yet
1 participant
@mikerobe

mikerobe commented Apr 1, 2013

Suppose someone adds directory('.') to connect(). It'll pass the check for non-empty root and if a requestor asks for /../../../../../../, it'll also pass the check for malicious path because path.indexOf(root) is true since the path starts with ..

So creating directory('.') in your middleware stack has the unintended result of giving someone listing rights on your whole computer (subject to permissions of web server user).

The directory middleware should probably resolve the root parameter to an absolute path before using it.

@mikerobe

This comment has been minimized.

Show comment
Hide comment
@mikerobe

mikerobe Apr 1, 2013

The easiest solution to this is to change line 56 in directory.js from

    , root = normalize(root);

to

    , root = path.resolve(root);

which normalizes and makes the path absolute.

Incidentally, it might also be a good idea to allow disabling the following of symlinks (although this would require doing lstat on every path component after the root)

mikerobe commented Apr 1, 2013

The easiest solution to this is to change line 56 in directory.js from

    , root = normalize(root);

to

    , root = path.resolve(root);

which normalizes and makes the path absolute.

Incidentally, it might also be a good idea to allow disabling the following of symlinks (although this would require doing lstat on every path component after the root)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment