Preventing circumvention of StaticURLParser and PkgResourcesParser's root directories.

Ensuring fix with tests

4 files changed, 69 insertions, 6 deletions

diff --git a/docs/news.txt b/docs/news.txt
index 2944d36..7162570 100644
--- a/docs/news.txt
+++ b/docs/news.txt
@@ -19,6 +19,9 @@ NEWS
 * Fixed ``paste.urlparser`` classes to handle quoted characters (e.g.
   %20) in URL paths.
 
+* Fixed a security vulnerability in ``paste.urlparser``'s StaticURLParser
+  and PkgResourcesParser when running under paste's httpserver.
+
 0.9.3
 -----
 
diff --git a/paste/urlparser.py b/paste/urlparser.py
index 8ba52a8..a404d31 100644
--- a/paste/urlparser.py
+++ b/paste/urlparser.py
@@ -414,10 +414,13 @@ class StaticURLParser(object):
     """
     # @@: Should URLParser subclass from this?
 
-    def __init__(self, directory):
+    def __init__(self, directory, root_directory=None):
         if os.path.sep != '/':
             directory = directory.replace(os.path.sep, '/')
         self.directory = directory
+        self.root_directory = root_directory
+        if root_directory is not None:
+            self.root_directory = os.path.normpath(self.root_directory)
 
     def __call__(self, environ, start_response):
         path_info = environ.get('PATH_INFO', '')
@@ -429,12 +432,18 @@ class StaticURLParser(object):
         else:
             # Handle quoted chars (e.g. %20)
             filename = urllib.unquote(request.path_info_pop(environ))
-        full = os.path.join(self.directory, filename)
+        full = os.path.normpath(os.path.join(self.directory, filename))
+        if self.root_directory is not None and not full.startswith(self.root_directory):
+            # Out of bounds
+            return self.not_found(environ, start_response)
         if not os.path.exists(full):
             return self.not_found(environ, start_response)
         if os.path.isdir(full):
             # @@: Cache?
-            return self.__class__(full)(environ, start_response)
+            child_root = self.root_directory is not None and \
+                self.root_directory or self.directory
+            return self.__class__(full, root_directory=child_root)(environ,
+                                                                   start_response)
         if environ.get('PATH_INFO') and environ.get('PATH_INFO') != '/':
             return self.error_extra_path(environ, start_response)
         if_none_match = environ.get('HTTP_IF_NONE_MATCH')
@@ -489,7 +498,7 @@ def make_static(global_conf, document_root):
 
 class PkgResourcesParser(StaticURLParser):
 
-    def __init__(self, egg_or_spec, resource_name, manager=None):
+    def __init__(self, egg_or_spec, resource_name, manager=None, root_resource=None):
         if isinstance(egg_or_spec, (str, unicode)):
             self.egg = pkg_resources.get_distribution(egg_or_spec)
         else:
@@ -498,6 +507,9 @@ class PkgResourcesParser(StaticURLParser):
         if manager is None:
             manager = pkg_resources.ResourceManager()
         self.manager = manager
+        self.root_resource = root_resource
+        if root_resource is not None:
+            self.root_resource = os.path.normpath(self.root_resource)
 
     def __repr__(self):
         return '<%s for %s:%r>' % (
@@ -515,12 +527,18 @@ class PkgResourcesParser(StaticURLParser):
         else:
             # Handle quoted chars (e.g. %20)
             filename = urllib.unquote(request.path_info_pop(environ))
-        resource = self.resource_name + '/' + filename
+        resource = os.path.normpath(self.resource_name + '/' + filename)
+        if self.root_resource is not None and not resource.startswith(self.root_resource):
+            # Out of bounds
+            return self.not_found(environ, start_response)
         if not self.egg.has_resource(resource):
             return self.not_found(environ, start_response)
         if self.egg.resource_isdir(resource):
             # @@: Cache?
-            return self.__class__(self.egg, resource, self.manager)(environ, start_response)
+            child_root = self.root_resource is not None and self.root_resource or \
+                self.resource_name
+            return self.__class__(self.egg, resource, self.manager,
+                                  root_resource=child_root)(environ, start_response)
         if environ.get('PATH_INFO') and environ.get('PATH_INFO') != '/':
             return self.error_extra_path(environ, start_response)
         
diff --git a/tests/test_urlparser.py b/tests/test_urlparser.py
index cfad691..5789c86 100644
--- a/tests/test_urlparser.py
+++ b/tests/test_urlparser.py
@@ -1,6 +1,7 @@
 import os
 from paste.urlparser import *
 from paste.fixture import *
+from pkg_resources import get_distribution
 
 def path(name):
     return os.path.join(os.path.dirname(os.path.abspath(__file__)),
@@ -37,6 +38,11 @@ def test_find_file():
     res = app.get('/dir%20with%20spaces/test%204.html')
     assert 'test 4' in res
     assert res.header('content-type') == 'text/html'
+    # Ensure only data under the app's root directory is accessible
+    res = app.get('/../secured.txt', status=404)
+    res = app.get('/dir with spaces/../../secured.txt', status=404)
+    res = app.get('/%2e%2e/secured.txt', status=404)
+    res = app.get('/dir%20with%20spaces/%2e%2e/%2e%2e/secured.txt', status=404)
 
 def test_deep():
     app = make_app('deep')
@@ -103,6 +109,11 @@ def test_static_parser():
     assert res.body.strip() == 'test 4'
     res = testapp.get('/dir%20with%20spaces/test%204.html')
     assert res.body.strip() == 'test 4'
+    # Ensure only data under the app's root directory is accessible
+    res = testapp.get('/../secured.txt', status=404)
+    res = testapp.get('/dir with spaces/../../secured.txt', status=404)
+    res = testapp.get('/%2e%2e/secured.txt', status=404)
+    res = testapp.get('/dir%20with%20spaces/%2e%2e/%2e%2e/secured.txt', status=404)
     
 def test_egg_parser():
     app = PkgResourcesParser('Paste', 'paste')
@@ -115,3 +126,33 @@ def test_egg_parser():
     res = testapp.get('/util/classinit', status=404)
     res = testapp.get('/util', status=301)
     res = testapp.get('/util/classinit.py/foo', status=400)
+    
+    # Find a readable file in the Paste pkg's root directory (or upwards the
+    # directory tree). Ensure it's not accessible via the URLParser
+    unreachable_test_file = None
+    search_path = pkg_root_path = get_distribution('Paste').location
+    level = 0
+    # This test might break if there's no readable files in the pkg's root
+    # directory (this is likely when Paste is installed as a .egg in
+    # site-packages). Traverse up the directory tree until one is found
+    while unreachable_test_file is None and \
+            os.path.normpath(search_path) != os.path.sep:
+        for file in os.listdir(search_path):
+            full_path = os.path.join(search_path, file)
+            if os.path.isfile(full_path) and os.access(full_path, os.R_OK):
+                unreachable_test_file = file
+                break
+
+        search_path = os.path.dirname(search_path)
+        level += 1
+    assert unreachable_test_file is not None, \
+           'test_egg_parser requires a readable file in a parent dir of the\n' \
+           'Paste pkg\'s root dir:\n%s' \
+           % pkg_root_path
+
+    unreachable_path = '/' + '../'*level + unreachable_test_file
+    unreachable_path_quoted = '/' + '%2e%2e/'*level + unreachable_test_file
+    res = testapp.get(unreachable_path, status=404)
+    res = testapp.get('/util/..' + unreachable_path, status=404)
+    res = testapp.get(unreachable_path_quoted, status=404)
+    res = testapp.get('/util/%2e%2e' + unreachable_path_quoted, status=404)    
diff --git a/tests/urlparser_data/secured.txt b/tests/urlparser_data/secured.txt
new file mode 100644
index 0000000..72b11b0
--- /dev/null
+++ b/tests/urlparser_data/secured.txt
@@ -0,0 +1 @@
+secured