Commit b8ba73cd authored by Aymeric Augustin's avatar Aymeric Augustin
Browse files

Raised SuspiciousFileOperation in safe_join. 

Added a test for the condition safe_join is designed to prevent.

Previously, a generic ValueError was raised. It was impossible to tell
an intentional exception raised to implement safe_join's contract from
an unintentional exception caused by incorrect inputs or unexpected
conditions. That resulted in bizarre exception catching patterns, which
this patch removes.

Since safe_join is a private API and since the change is unlikely to
create security issues for users who use it anyway -- at worst, an
uncaught SuspiciousFileOperation exception will bubble up -- it isn't
documented.
parent 40ba6f21

django/core/files/storage.py

+1 −5
Original line number Diff line number Diff line
@@ -277,11 +277,7 @@ class FileSystemStorage(Storage): 
        return directories, files



    def path(self, name):

        try:

            path = safe_join(self.location, name)

        except ValueError:

            raise SuspiciousFileOperation("Attempted access to '%s' denied." % name)

        return os.path.normpath(path)

        return safe_join(self.location, name)



    def size(self, name):

        return os.path.getsize(self.path(name))

django/template/loaders/app_directories.py

+4 −5
Original line number Diff line number Diff line
@@ -8,6 +8,7 @@ import sys 


from django.apps import apps

from django.conf import settings

from django.core.exceptions import SuspiciousFileOperation

from django.template.base import TemplateDoesNotExist

from django.template.loader import BaseLoader

from django.utils._os import safe_join
@@ -47,11 +48,9 @@ class Loader(BaseLoader): 
        for template_dir in template_dirs:

            try:

                yield safe_join(template_dir, template_name)

            except UnicodeDecodeError:

                # The template dir name was a bytestring that wasn't valid UTF-8.

                raise

            except ValueError:

                # The joined path was located outside of template_dir.

            except SuspiciousFileOperation:

                # The joined path was located outside of this template_dir

                # (it might be inside another one, so this isn't fatal).

                pass



    def load_template_source(self, template_name, template_dirs=None):

django/template/loaders/filesystem.py

+4 −7
Original line number Diff line number Diff line
@@ -3,6 +3,7 @@ Wrapper for loading templates from the filesystem. 
"""



from django.conf import settings

from django.core.exceptions import SuspiciousFileOperation

from django.template.base import TemplateDoesNotExist

from django.template.loader import BaseLoader

from django.utils._os import safe_join
@@ -22,13 +23,9 @@ class Loader(BaseLoader): 
        for template_dir in template_dirs:

            try:

                yield safe_join(template_dir, template_name)

            except UnicodeDecodeError:

                # The template dir name was a bytestring that wasn't valid UTF-8.

                raise

            except ValueError:

                # The joined path was located outside of this particular

                # template_dir (it might be inside another one, so this isn't

                # fatal).

            except SuspiciousFileOperation:

                # The joined path was located outside of this template_dir

                # (it might be inside another one, so this isn't fatal).

                pass



    def load_template_source(self, template_name, template_dirs=None):

django/utils/_os.py

+4 −2
Original line number Diff line number Diff line
@@ -4,6 +4,7 @@ import sys 
import tempfile

from os.path import join, normcase, normpath, abspath, isabs, sep, dirname



from django.core.exceptions import SuspiciousFileOperation

from django.utils.encoding import force_text

from django.utils import six
@@ -77,8 +78,9 @@ def safe_join(base, *paths): 
    if (not normcase(final_path).startswith(normcase(base_path + sep)) and

            normcase(final_path) != normcase(base_path) and

            dirname(normcase(base_path)) != normcase(base_path)):

        raise ValueError('The joined path (%s) is located outside of the base '

                         'path component (%s)' % (final_path, base_path))

        raise SuspiciousFileOperation(

            'The joined path ({}) is located outside of the base path '

            'component ({})'.format(final_path, base_path))

    return final_path

tests/utils_tests/test_os_utils.py

+5 −0
Original line number Diff line number Diff line 
import os

import unittest



from django.core.exceptions import SuspiciousFileOperation

from django.utils._os import safe_join
@@ -24,3 +25,7 @@ class SafeJoinTests(unittest.TestCase): 
            path,

            os.path.sep,

        )



    def test_parent_path(self):

        with self.assertRaises(SuspiciousFileOperation):

            safe_join("/abc/", "../def")