Pull requests

#20 Merged
Repository
alanjds alanjds
Branch
default
Repository
david david
Branch
default

SuspiciousOperation rasing if AWS_LOCATION ends with '/'

Author
  1. Alan Justino
Reviewers
Description

Looks like safe_join tried to guard against paths like '/media' + 'other_folder' paths, but it is safe by being sure that '/media' ends with '/'.

A real problem could be '/media/' + 'path/../../media-parent-folder', which I tried to fix.

(btw, sorry by the misspelling at some commit title)

Comments (3)

  1. Ian Lewis

    I think you tried to do too much without really looking at why safe join was failing. It was failing because the base path ending in '/' caused the final comparison to be incorrect.

    Try something like this:

    def safe_join(base, *paths):
        """
        A version of django.utils._os.safe_join for S3 paths.
    
        Joins one or more path components to the base path component intelligently.
        Returns a normalized version of the final path.
    
        The final path must be located inside of the base path component (otherwise
        a ValueError is raised).
    
        Paths outside the base path indicate a possible security sensitive operation.
        """
        from urlparse import urljoin
        base_path = force_unicode(base)
        base_path = base_path.rstrip('/')
        paths = [force_unicode(p) for p in paths]
        final_path = urljoin(base_path + "/", *paths)
        # Ensure final_path starts with base_path and that the next character after
        # the final path is '/' (or nothing, in which case final_path must be
        # equal to base_path).
        base_path_len = len(base_path)
        if not final_path.startswith(base_path) \
           or final_path[base_path_len:base_path_len+1] not in ('', '/'):
            raise ValueError('the joined path is located outside of the base path'
                             ' component')
        return final_path
    

    I just stripped the '/' characters from the left of the base_path.