1
\$\begingroup\$

I made a simple web-based file browser with Flask. It also shows some metadata of a file. I have a few questions regarding security though. Is it possible for an attacker to escape out of the preset root directory? Are there any ways to make my code more readable and/or more efficient? Would love to hear some feedback to improve my program! :)

app.py:

from flask import Flask, render_template
import os
import stat
app = Flask(__name__)
# Make the WSGI interface available at the top level so wfastcgi can get it.
wsgi_app = app.wsgi_app
FILE_SYSTEM_ROOT = "D:\Test123"
@app.route('/')
def index():
 return render_template('index.html')
@app.route('/browser')
def browse():
 itemList = os.listdir(FILE_SYSTEM_ROOT)
 return render_template('browse.html', itemList=itemList)
@app.route('/browser/<path:urlFilePath>')
def browser(urlFilePath):
 nestedFilePath = os.path.join(FILE_SYSTEM_ROOT, urlFilePath)
 if os.path.isdir(nestedFilePath):
 itemList = os.listdir(nestedFilePath)
 fileProperties = {"filepath": nestedFilePath}
 if not urlFilePath.startswith("/"):
 urlFilePath = "/" + urlFilePath
 return render_template('browse.html', urlFilePath=urlFilePath, itemList=itemList)
 if os.path.isfile(nestedFilePath):
 fileProperties = {"filepath": nestedFilePath}
 sbuf = os.fstat(os.open(nestedFilePath, os.O_RDONLY)) #Opening the file and getting metadata
 fileProperties['type'] = stat.S_IFMT(sbuf.st_mode) 
 fileProperties['mode'] = stat.S_IMODE(sbuf.st_mode) 
 fileProperties['mtime'] = sbuf.st_mtime 
 fileProperties['size'] = sbuf.st_size 
 if not urlFilePath.startswith("/"):
 urlFilePath = "/" + urlFilePath
 return render_template('file.html', currentFile=nestedFilePath, fileProperties=fileProperties)
 return 'something bad happened'
if __name__ == '__main__':
 app.run(host='0.0.0.0', port=80)

browse.html:

<!DOCTYPE html>
<html lang="en" xmlns="http://www.w3.org/1999/xhtml">
<head>
 <meta charset="utf-8" />
 <title></title>
</head>
<body>
 <ul>
 {% for item in itemList %}
 <li><a href="/browser{{ urlFilePath }}/{{ item }}">{{ item }}</a></li>
 {% endfor %}
 </ul>
</body>
</html>

file.html:

<!DOCTYPE html>
<html lang="en" xmlns="http://www.w3.org/1999/xhtml">
<head>
 <meta charset="utf-8" />
 <title></title>
</head>
<body>
 <p>Current file: {{ currentFile }}</p>
 <p>
 <table>
 {% for key, value in fileProperties.items() %}
 <tr>
 <td>{{ key }}</td>
 <td>{{ value }}</td>
 </tr>
 {% endfor %}
 </table>
 </p>
</body>
</html>

The output when browsing looks something like this: browse.html example

The output with the metadata of a file looks like this: file.html example

Zeta
19.6k2 gold badges57 silver badges90 bronze badges
asked Feb 27, 2019 at 20:20
\$\endgroup\$
2
  • 2
    \$\begingroup\$ Food for thoughts: what would happen if I browsed to htrp://localhost/browser/../an/other/path.txt? \$\endgroup\$ Commented Feb 27, 2019 at 22:38
  • 2
    \$\begingroup\$ I have rolled back the last edit. Please see What to do when someone answers . \$\endgroup\$ Commented Mar 2, 2019 at 10:55

2 Answers 2

1
\$\begingroup\$

@Matthias has already mentioned this in the comments, and he is correct.

The code suffers from a Directory Traversal vulnerability

You can filter this in 2 ways:

  1. Use os.path.realpath and check if the resolved path is different

    def check_lfi(file_path):
 return os.path.realpath(file_path) != file_path

  2. Use regex to filter out bad (../) Paths

    import re
DIR_TRAVERSAL = re.compile(r"\.\.\/")
def check_lfi(file_path):
 return bool(DIR_TRAVERSAL.search(file_path))

My advice would be to use the first example, since blacklisting can be error prone (Unicode for instance)

answered Feb 28, 2019 at 10:35
\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the hint, I updated my original post with the fix. I tested it out with curl and directory traversals aren't possible anymore. \$\endgroup\$ Commented Mar 2, 2019 at 10:47
1
\$\begingroup\$

I changed my code with the feedback from Ludiposed. This is what it looks like:

@app.route('/browser/<path:urlFilePath>')
def browser(urlFilePath):
 nestedFilePath = os.path.join(FILE_SYSTEM_ROOT, urlFilePath)
 nestedFilePath = nestedFilePath.replace("/", "\\")
 if os.path.realpath(nestedFilePath) != nestedFilePath:
 return "no directory traversal please."
 if os.path.isdir(nestedFilePath):
 itemList = os.listdir(nestedFilePath)
 fileProperties = {"filepath": nestedFilePath}
 if not urlFilePath.startswith("/"):
 urlFilePath = "/" + urlFilePath
 return render_template('browse.html', urlFilePath=urlFilePath, itemList=itemList)
 if os.path.isfile(nestedFilePath):
 fileProperties = {"filepath": nestedFilePath}
 sbuf = os.fstat(os.open(nestedFilePath, os.O_RDONLY)) #Opening the file and getting metadata
 fileProperties['type'] = stat.S_IFMT(sbuf.st_mode) 
 fileProperties['mode'] = stat.S_IMODE(sbuf.st_mode) 
 fileProperties['mtime'] = sbuf.st_mtime 
 fileProperties['size'] = sbuf.st_size 
 if not urlFilePath.startswith("/"):
 urlFilePath = "/" + urlFilePath
 return render_template('file.html', currentFile=nestedFilePath, fileProperties=fileProperties)
 return 'something bad happened'

I changed some things:

  • The forward slashes get replaced by backward slahes, since i'm developing in a Windows enviroment.
  • The canonical filepath should be the same as the requested filepath, this filters out ../ and other directory traversal methods from the url.
  • The new code has been tested with curl, and is safe.
answered Mar 2, 2019 at 11:37
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.