deamon deamon - 1 year ago 98
Python Question

Does my code prevent directory traversal?

Is the following code snippet from a Python WSGI app safe from directory traversal? It reads a file name passed as parameter and returns the named file.

file_name = request.path_params["file"]
file = open(file_name, "rb")
mime_type = mimetypes.guess_type(file_name)[0]
start_response(status.OK, [('Content-Type', mime_type)])
return file

I mounted the app under
and sent requests with the URLs
. But none of my attempts delivered the (existing) file. So is my code already safe from directory traversal?

New approach

It seems like the following code prevents directory traversal:

file_name = request.path_params["file"]
absolute_path = os.path.join(self.base_directory, file_name)
normalized_path = os.path.normpath(absolute_path)

# security check to prevent directory traversal
if not normalized_path.startswith(self.base_directory):
raise IOError()

file = open(normalized_path, "rb")
mime_type = mimetypes.guess_type(normalized_path)[0]
start_response(status.OK, [('Content-Type', mime_type)])
return file

Answer Source

Your code does not prevent directory traversal. You can guard against this with the os.path module.

>>> import os.path
>>> os.curdir
>>> startdir = os.path.abspath(os.curdir)
>>> startdir

startdir is now an absolute path where you don't want to allow the path to go outside of. Now let's say we get a filename from the user and they give us the malicious /etc/passwd.

>>> filename = "/etc/passwd"
>>> requested_path = os.path.relpath(filename, startdir)
>>> requested_path
>>> requested_path = os.path.abspath(requested_path)
>>> requested_path

We have now converted their path into an absolute path relative to our starting path. Since this wasn't in the starting path, it doesn't have the prefix of our starting path.

>>> os.path.commonprefix([requested_path, startdir])

You can check for this in your code. If the commonprefix function returns a path that doesn't start with startdir, then the path is invalid and you should not return the contents.

The above can be wrapped to a static method like so:

import os 

def is_directory_traversal(file_name):
    current_directory = os.path.abspath(os.curdir)
    requested_path = os.path.relpath(file_name, start=current_directory)
    requested_path = os.path.abspath(requested_path)
    common_prefix = os.path.commonprefix([requested_path, current_directory])
    return common_prefix != current_directory
Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download