Refactor out sanitize_path

website/photos/tests.py

+import doctest
+
+from . import views
+
+
+def load_tests(loader, tests, ignore):
+    """
+    Load all tests in this module
+    """
+    # Adds the doctests in views
+    tests.addTests(doctest.DocTestSuite(views))
+    return tests

website/photos/views.py

@@ -11,7 +11,6 @@ from sendfile import sendfile
 from zipfile import ZipFile
 from tempfile import gettempdir
 
-from utils.snippets import sanitize_path
 from utils.views import _private_thumbnails_unauthed
 
 from .models import Album
@@ -109,11 +108,21 @@ def shared_album(request, slug, token):
 
 
 def _download(request, original_path):
-    """This function provides a layer of indirection for shared albums"""
+    """This function provides a layer of indirection for shared albums
+
+    Checks for some path traversal:
+
+    >>> from django.test import RequestFactory
+    >>> r = RequestFactory().get('/photos/download/../../../../../etc/passwd')
+    >>> _download(r, '../../../../../../../etc/passwd')  #doctest: +ELLIPSIS
+    Traceback (most recent call last):
+        ...
+    django.core.exceptions.SuspiciousFileOperation: ...
+    """
     photopath = os.path.join(settings.MEDIA_ROOT, 'photos')
 
-    path = sanitize_path(original_path)
-    path = os.path.normpath(os.path.join(photopath, *path.split('/')[1:]))
+    path = os.path.normpath(
+        os.path.join(photopath, *original_path.split('/')[1:]))
 
     if not os.path.commonprefix([photopath, path]).startswith(photopath):
         raise SuspiciousFileOperation(

website/utils/snippets.py

@@ -30,42 +30,6 @@ def datetime_to_lectureyear(date):
     return date.year
 
 
-def sanitize_path(path):
-    r"""
-    Cleans up an insecure path, i.e. against directory traversal.
-
-    Still use os.path.commonprefix to check if the target is as expected
-
-    This code is partially copied from ``django.views.static``.
-
-    >>> sanitize_path('//////')
-    ''
-    >>> sanitize_path('////test//')
-    'test'
-    >>> sanitize_path('../../../test/')
-    'test'
-    >>> sanitize_path('../.././test/')
-    'test'
-    >>> sanitize_path(r'..\..\..\test')
-    'test'
-
-    """
-    path = os.path.normpath(unquote(path).replace('\\', '/'))
-    path = path.lstrip('/')
-    newpath = ''
-    for part in path.split('/'):
-        if not part:
-            # Strip empty path components.
-            continue
-        drive, part = os.path.splitdrive(part)
-        head, part = os.path.split(part)
-        if part in (os.curdir, os.pardir):
-            # Strip '.' and '..' in path.
-            continue
-        newpath = os.path.join(newpath, part)
-    return newpath
-
-
 if __name__ == "__main__":
     import doctest
     doctest.testmod()

website/utils/views.py

 import os
 
 from PIL import Image, ImageOps
+from django.core.exceptions import SuspiciousFileOperation
 from django.conf import settings
 from django.contrib.auth.decorators import login_required
 from django.http import Http404
@@ -9,15 +10,21 @@ from django.urls import reverse
 from django.utils.http import urlunquote
 from sendfile import sendfile
 
-from .snippets import sanitize_path
 
+def _private_thumbnails_unauthed(request, size_fit, original_path):
+    """
+    Serve thumbnails from the filesystem
 
-def _private_thumbnails_unauthed(request, size_fit, path):
-    """This layer of indirection makes it possible to make exceptions
+    This layer of indirection makes it possible to make exceptions
     to the authentication requirements for thumbnails, e.g. when sharing
-    photo albums with external parties using access tokens."""
-    path = sanitize_path(path)
-    path = os.path.join(settings.MEDIA_ROOT, 'thumbnails', size_fit, path)
+    photo albums with external parties using access tokens.
+    """
+    thumbpath = os.path.join(settings.MEDIA_ROOT, 'thumbnails', size_fit)
+    path = os.path.normpath(os.path.join(thumbpath, original_path))
+    if not os.path.commonprefix([thumbpath, path]).startswith(thumbpath):
+        raise SuspiciousFileOperation(
+            "Path traversal detected: someone tried to download "
+            "{}, input: {}".format(path, original_path))
     if not os.path.isfile(path):
         raise Http404("Thumbnail not found.")
     return sendfile(request, path)