When serving files, access to entire filesystem is possible

[DEGoodmanWilson]

This is a security vulnerability exposed by the built-in file serving helper function router::serve_files(). It allows attackers to craft requests that begin with /.., which means they can read arbitrary files in the filesystem.

For example, run the following program from your home directory in OS X:

#include <luna/luna.h>
int main(void)
{
    luna::server server;
    auto router = server.create_router("/");
    router->serve_files("/", path);
    server.start(port);
    return 0;
}

then in the terminal

telnet localhost 8273

and issue the following HTTP request

GET /../../etc/passwd HTTP/1.1

And you will get the contents of /etc/passwd. No good.

---

Fix: Make router::serve_files() a bit smarter, by filtering out any initial .. in the path requested.

[arpit2297]

Hi! I can take this up

[destoer]

Hi by initial ../ did you mean ones before the first directory, or all of them because directory traversal can also happen with requests like this GET /public/../../main.cpp HTTP/1.1

https://imgur.com/a/hWtlMsx

[DEGoodmanWilson]

I had meant .. before the first directory. However, now that you mention it, we should probably be reasonably smart about this. Ideally, we should allow .. path navigation within the served folder, but to 404 when the final resolved path takes us out of the served folder. In other words, if we are serving /something/something/public on / then

• GET /file.txt should succeed — it resolves to /something/something/public/file.txt
• GET /foobar/../file.txt should succeed — it resolves to /something/something/public/file.txt
• GET /../file.txtshould fail — it resolves to /something/something/file.txt, outside of the public folder
• GET /foobar/../../file.txt should fail — it resolves to /something/something/file.txt, outside of the public folder

[destoer]

Yes ideally it should only reject paths outside of the public folder and not just outright ban .. in the requested path.

A method along these lines may be suitable for detecting bad paths simply parse each token between a pair of / if its a .. sub one from depth else add 1 to depth, if depth < 0 we have gone behind the file root return an error 404 or maybe the index.

https://pastebin.com/ZPpsncWb

[DEGoodmanWilson]

That's quite close to what I'm thinking of. I think we should do something a little more sophisticated, and attempt to resolve the path, so that, for example: foobar/../test.txt becomes test.txt

It could use the same state machine you describe, only instead of incrementing or decrementing an int, it is pushing and popping paths from a stack.

So: See a .., pop what ever's on the stack (leaving it empty if it's empty, of course) See anything else: push the folder name onto the stack

Then iterate over the stack, concatenating what paths remain with '/'.

[arpit2297]

At any point, if we encounter a .. and the stack is empty, should we return a 404 (since a file outside the root directory is being requested). I like the stack idea since it not only fixes the vulnerability, but also creates a cleaner path to work with.

[DEGoodmanWilson]

Yeah, we can totally shortcut in that case.