Merge lp:~dooferlad/linaro-license-protection/bug1187726 into lp:~linaro-automation/linaro-license-protection/trunk

[James Tunnicliffe]

[Milo Casagrande (milo)]

Looks OK to me.
There is only one typo I guess:

+ """os.path.join with with check that result is inside base_path.

s/with with/with
In case, fix when merging.

review: Approve

=== modified file 'license_protected_downloads/tests/test_views.py'
--- license_protected_downloads/tests/test_views.py	2013-05-27 15:35:04 +0000
+++ license_protected_downloads/tests/test_views.py	2013-06-05 11:32:29 +0000
@@ -821,6 +821,24 @@
             self.assertEqual(response.status_code, 200)
             self.assertContains(response, 'test CSS')
 
+    def test_path_to_root(self):
+        response = self.client.get("http://testserver//", follow=True)
+
+        # Shouldn't be able to escape served paths...
+        self.assertEqual(response.status_code, 404)
+
+    def test_path_to_dir_above(self):
+        response = self.client.get("http://testserver/../", follow=True)
+
+        # Shouldn't be able to escape served paths...
+        self.assertEqual(response.status_code, 404)
+
+    def test_path_to_dir_above2(self):
+        response = self.client.get("http://testserver/..", follow=True)
+
+        # Shouldn't be able to escape served paths...
+        self.assertEqual(response.status_code, 404)
+
 
 class HowtoViewTests(BaseServeViewTest):
     def test_no_howtos(self):

=== modified file 'license_protected_downloads/views.py'
--- license_protected_downloads/views.py	2013-06-04 18:50:56 +0000
+++ license_protected_downloads/views.py	2013-06-05 11:32:29 +0000
@@ -132,10 +132,31 @@
     return listing
 
 
+def safe_path_join(base_path, *paths):
+    """os.path.join with with check that result is inside base_path.
+
+    Checks that the generated path doesn't end up outside the target
+    directory, so server accesses stay where we expect them.
+    """
+
+    target_path = os.path.join(base_path, *paths)
+
+    if not target_path.startswith(base_path):
+        return None
+
+    if not os.path.normpath(target_path) == target_path.rstrip("/"):
+        return None
+
+    return target_path
+
+
 def test_path(path):
 
     for basepath in settings.SERVED_PATHS:
-        fullpath = os.path.join(basepath, path)
+        fullpath = safe_path_join(basepath, path)
+        if fullpath is None:
+            return None
+
         if os.path.isfile(fullpath):
             return ("file", fullpath)
         if os.path.isdir(fullpath):