mcmt
mcmt

Reputation: 33

Is using os.path.abspath to validate an untrusted filename's location secure?

I don't think I'm missing anything. Then again I'm kind of a newbie.

def GET(self, filename):
    name = urllib.unquote(filename)
    full = path.abspath(path.join(STATIC_PATH, filename))
    #Make sure request is not tricksy and tries to get out of
    #the directory, e.g. filename = "../.ssh/id_rsa". GET OUTTA HERE
    assert full[:len(STATIC_PATH)] == STATIC_PATH, "bad path"
    return open(full).read()

Edit: I realize this will return the wrong HTTP error code if the file doesn't exist (at least under web.py). I will fix this.

Upvotes: 3

Views: 622

Answers (3)

Jason Baker
Jason Baker

Reputation: 198727

Generally speaking, dealing with these kinds of issues is a huge pain. There's always some tricky way of combining path names to create something a programmer didn't expect or plan for.

A better solution is usually to make sure that your webserver is running under a user that only has access to the files it needs to have access to. I think a simple solution to the ssh issue is not to allow this user SSH access. If you really need to log in under this user, I'd consider logging in under a different account and su-ing to it.

Upvotes: 1

Alex Martelli
Alex Martelli

Reputation: 882381

os.path.abspath, per se, is quite secure. Your assert check will be compiled away if run under python -O, which is one risk. If your STATIC_PATH does not end with the proper directory separator, you might accidentally allow a path which just happens to have it as a prefix -- e.g., if STATIC_PATH is /foo/bar, you'd erroneously accept a file living under /foo/barbie/ (so, unless STATIC_PATH does end with the separator character, you need a slightly stricter check for strong assurance).

Upvotes: 6

Ignacio Vazquez-Abrams
Ignacio Vazquez-Abrams

Reputation: 799190

Make sure that either STATIC_PATH ends with a directory separator, or that the character following it in full is such.

Upvotes: 0

Related Questions