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

make safe_join behave like os.path.join with *args #1730

Merged
merged 2 commits into from
Jun 4, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ Version 0.12
- the cli command now responds to `--version`.
- Mimetype guessing for ``send_file`` has been removed, as per issue ``#104``.
See pull request ``#1849``.
- Make ``flask.safe_join`` able to join multiple paths like ``os.path.join``
(pull request ``#1730``).

Version 0.11.1
--------------
Expand Down
32 changes: 18 additions & 14 deletions flask/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -563,8 +563,9 @@ def send_file(filename_or_fp, mimetype=None, as_attachment=False,
return rv


def safe_join(directory, filename):
"""Safely join `directory` and `filename`.
def safe_join(directory, *pathnames):
"""Safely join `directory` and zero or more untrusted `pathnames`
components.

Example usage::

Expand All @@ -574,20 +575,23 @@ def wiki_page(filename):
with open(filename, 'rb') as fd:
content = fd.read() # Read and process the file content...

:param directory: the base directory.
:param filename: the untrusted filename relative to that directory.
:raises: :class:`~werkzeug.exceptions.NotFound` if the resulting path
would fall out of `directory`.
:param directory: the trusted base directory.
:param pathnames: the untrusted pathnames relative to that directory.
:raises: :class:`~werkzeug.exceptions.NotFound` if one or more passed
paths fall out of its boundaries.
"""
filename = posixpath.normpath(filename)
for sep in _os_alt_seps:
if sep in filename:
for filename in pathnames:
if filename != '':
filename = posixpath.normpath(filename)
for sep in _os_alt_seps:
if sep in filename:
raise NotFound()
if os.path.isabs(filename) or \
filename == '..' or \
filename.startswith('../'):
raise NotFound()
if os.path.isabs(filename) or \
filename == '..' or \
filename.startswith('../'):
raise NotFound()
return os.path.join(directory, filename)
directory = os.path.join(directory, filename)
return directory


def send_from_directory(directory, filename, **options):
Expand Down
44 changes: 43 additions & 1 deletion tests/test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import datetime
import flask
from logging import StreamHandler
from werkzeug.exceptions import BadRequest
from werkzeug.exceptions import BadRequest, NotFound
from werkzeug.http import parse_cache_control_header, parse_options_header
from werkzeug.http import http_date
from flask._compat import StringIO, text_type
Expand Down Expand Up @@ -722,3 +722,45 @@ def generate():
rv = c.get('/?name=World')
assert rv.data == b'Hello World!'
assert called == [42]


class TestSafeJoin(object):

def test_safe_join(self):
# Valid combinations of *args and expected joined paths.
passing = (
(('a/b/c', ), 'a/b/c'),
(('/', 'a/', 'b/', 'c/', ), '/a/b/c'),
(('a', 'b', 'c', ), 'a/b/c'),
(('/a', 'b/c', ), '/a/b/c'),
(('a/b', 'X/../c'), 'a/b/c', ),
(('/a/b', 'c/X/..'), '/a/b/c', ),
# If last path is '' add a slash
(('/a/b/c', '', ), '/a/b/c/', ),
# Preserve dot slash
(('/a/b/c', './', ), '/a/b/c/.', ),
(('a/b/c', 'X/..'), 'a/b/c/.', ),
# Base directory is always considered safe
(('../', 'a/b/c'), '../a/b/c'),
(('/..', ), '/..'),
)

for args, expected in passing:
assert flask.safe_join(*args) == expected

def test_safe_join_exceptions(self):
# Should raise werkzeug.exceptions.NotFound on unsafe joins.
failing = (
# path.isabs and ``..'' checks
('/a', 'b', '/c'),
('/a', '../b/c', ),
('/a', '..', 'b/c'),
# Boundaries violations after path normalization
('/a', 'b/../b/../../c', ),
('/a', 'b', 'c/../..'),
('/a', 'b/../../c', ),
)

for args in failing:
with pytest.raises(NotFound):
print(flask.safe_join(*args))