Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

static_file endpoint doesn't love null byte #1761

Closed
Lothiraldan opened this issue Mar 28, 2016 · 7 comments
Closed

static_file endpoint doesn't love null byte #1761

Lothiraldan opened this issue Mar 28, 2016 · 7 comments

Comments

@Lothiraldan
Copy link

Hello,

while fuzzing my API, I think I've discovered a Flask issue. The static_file endpoint generate a 500 if the filename include a null byte (\x00).

Here is a minimal flask application:

from flask import Flask
app = Flask(__name__)


if __name__ == '__main__':
    app.run(debug=True)

I've tried launching it with either python 2 (2.7.11) or python 3 interpreter (3.5.1) with flask 0.10.1, then make this request:

import requests
requests.get('http://localhost:5000/static/\x00')

I get a 500, here is the traceback for python2:

Traceback (most recent call last):
  File "/usr/local/lib/python2.7/site-packages/flask/app.py", line 1836, in __call__
    return self.wsgi_app(environ, start_response)
  File "/usr/local/lib/python2.7/site-packages/flask/app.py", line 1820, in wsgi_app
    response = self.make_response(self.handle_exception(e))
  File "/usr/local/lib/python2.7/site-packages/flask/app.py", line 1403, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/local/lib/python2.7/site-packages/flask/app.py", line 1817, in wsgi_app
    response = self.full_dispatch_request()
  File "/usr/local/lib/python2.7/site-packages/flask/app.py", line 1477, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/usr/local/lib/python2.7/site-packages/flask/app.py", line 1381, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/local/lib/python2.7/site-packages/flask/app.py", line 1475, in full_dispatch_request
    rv = self.dispatch_request()
  File "/usr/local/lib/python2.7/site-packages/flask/app.py", line 1461, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/usr/local/lib/python2.7/site-packages/flask/helpers.py", line 823, in send_static_file
    cache_timeout=cache_timeout)
  File "/usr/local/lib/python2.7/site-packages/flask/helpers.py", line 613, in send_from_directory
    if not os.path.isfile(filename):
  File "/usr/local/Cellar/python/2.7.11/Frameworks/Python.framework/Versions/2.7/lib/python2.7/genericpath.py", line 37, in isfile
    st = os.stat(path)
TypeError: must be encoded string without NULL bytes, not unicode

And here is the traceback for python 3:

Traceback (most recent call last):
  File "/usr/local/lib/python3.5/site-packages/flask/app.py", line 1836, in __call__
    return self.wsgi_app(environ, start_response)
  File "/usr/local/lib/python3.5/site-packages/flask/app.py", line 1820, in wsgi_app
    response = self.make_response(self.handle_exception(e))
  File "/usr/local/lib/python3.5/site-packages/flask/app.py", line 1403, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/local/lib/python3.5/site-packages/flask/_compat.py", line 33, in reraise
    raise value
  File "/usr/local/lib/python3.5/site-packages/flask/app.py", line 1817, in wsgi_app
    response = self.full_dispatch_request()
  File "/usr/local/lib/python3.5/site-packages/flask/app.py", line 1477, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/usr/local/lib/python3.5/site-packages/flask/app.py", line 1381, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/local/lib/python3.5/site-packages/flask/_compat.py", line 33, in reraise
    raise value
  File "/usr/local/lib/python3.5/site-packages/flask/app.py", line 1475, in full_dispatch_request
    rv = self.dispatch_request()
  File "/usr/local/lib/python3.5/site-packages/flask/app.py", line 1461, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/usr/local/lib/python3.5/site-packages/flask/helpers.py", line 823, in send_static_file
    cache_timeout=cache_timeout)
  File "/usr/local/lib/python3.5/site-packages/flask/helpers.py", line 613, in send_from_directory
    if not os.path.isfile(filename):
  File "/usr/local/Cellar/python3/3.5.1/Frameworks/Python.framework/Versions/3.5/lib/python3.5/genericpath.py", line 30, in isfile
    st = os.stat(path)
ValueError: embedded null byte

The error is not exactly the same but isfile doesn't seems to love null bytes.

I've tried to fix it locally by adding this piece of code to detect null bytes in send_static_file (https://github.com/mitsuhiko/flask/blob/master/flask/helpers.py#L862):

if '\x00' in filename:
     raise RuntimeError("Null byte")

It seems to works with both python2 and python3, but I'm not sure what is the best response when the request include a null byte.

Here is the output of pip freeze if necessary:

Flask==0.10.1
itsdangerous==0.24
Jinja2==2.8
MarkupSafe==0.23
Werkzeug==0.11.5

I only tried on Mac OS X 10.11.4, I don't know if null byte are accepted in valid filename on other filesystems.

@ThiefMaster
Copy link
Member

I don't think NULs are acceptable in any paths since the functions dealing with paths expect NUL-terminated strings in the C APIs.

In any case, I'd raise a NotFound error instead of a RuntimeError.

@untitaker
Copy link
Contributor

Nullbyte should raise a 400 Bad Request since they're invalid characters in filenames.

@ThiefMaster
Copy link
Member

hm.. are they disallowed in URLs? if yes 400 makes more sense indeed. otherwise i'd go for 404 since it's none of the client's business whether the file is loaded from the filesystem (where NUL is disallowed) or somewhere else (where it might be valid)

@untitaker
Copy link
Contributor

I guess 404 is also a solution.

Anyway, this should be implemented by catching ValueError from the os.path.isfile call in the traceback. Not just checking for nullbytes, because there might be other rules for valid filenames on Windows.

@Lothiraldan
Copy link
Author

I can make a pull-request but what is the best way to return a 400 inside flask/helpers? abort?

@ThiefMaster
Copy link
Member

check existing code, but you could import BadRequest from werkzeug.exceptions and raise that

@chaosagent
Copy link
Contributor

Fixed in above pull request; this issue should be closed.

@davidism davidism closed this as completed Apr 2, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants