fix the detection of unacceptable symlinks (#89)

In #86 I forgot that we sometimes load resources from outside the `www_root` directory, for example Pando's `error.spt` file is located in the `project_root` directory instead.

aspen/http/resource.py

@@ -7,20 +7,26 @@
 from ..output import Output
 
 
-def open_resource(www_root, resource_path):
+def _is_subpath(path, root):
+    return path.startswith(root) and path[len(root)] == os.path.sep
+
+
+def open_resource(request_processor, resource_path):
     """Open a resource in read-only binary mode, after checking for symlinks.
 
     :raises AttemptedBreakout:
-        if the :arg:`resource_path` points to a file outside the :arg:`www_root`
+        if the :obj:`resource_path` points to a file outside of both the
+        :obj:`project_root` and :obj:`www_root` directories
 
     This function doesn't fully protect against attackers who have the ability
-    to create and delete symlinks inside the `www_root` whenever they want, but
-    it makes the attack more difficult and detectable.
+    to create and delete symlinks inside the `project_root` or `www_root`
+    whenever they want, but it makes the attack more difficult and detectable.
     """
-    if not os.path.isabs(resource_path):
-        resource_path = os.path.join(www_root, resource_path)
     real_path = os.path.realpath(resource_path)
-    if not real_path.startswith(www_root.rstrip(os.path.sep) + os.path.sep):
+    is_outside = not _is_subpath(real_path, request_processor.www_root)
+    if is_outside and request_processor.project_root:
+        is_outside &= not _is_subpath(real_path, request_processor.project_root)
+    if is_outside:
         raise AttemptedBreakout(resource_path, real_path)
     return open(real_path, 'rb')
 
@@ -38,7 +44,7 @@ def __init__(self, request_processor, fspath):
             request_processor.charset_static
         )
         if read_file:
-            with open_resource(request_processor.www_root, fspath) as f:
+            with open_resource(request_processor, fspath) as f:
                 raw = f.read()
         self.request_processor = request_processor
         self.fspath = fspath
@@ -61,7 +67,7 @@ def render(self, *ignored):
         """
         output = Output(media_type=self.media_type, charset=self.charset)
         if self.raw is None:
-            with open_resource(self.request_processor.www_root, self.fspath) as f:
+            with open_resource(self.request_processor, self.fspath) as f:
                 output.body = f.read()
         else:
             output.body = self.raw

aspen/simplates/simplate.py

@@ -107,7 +107,7 @@ def __init__(self, request_processor, fspath):
 
         self.renderers = {}         # mapping of media type to Renderer objects
         self.available_types = []   # ordered sequence of media types
-        with open_resource(request_processor.www_root, fspath) as fh:
+        with open_resource(request_processor, fspath) as fh:
             raw = fh.read()
         pages = self.parse_into_pages(_decode(raw))
         self.compile_pages(pages)

tests/test_resources.py

@@ -3,13 +3,18 @@
 from __future__ import print_function
 from __future__ import unicode_literals
 
+import os
+import sys
+from warnings import catch_warnings
 
+from aspen.exceptions import AttemptedBreakout, PossibleBreakout
+from aspen.http.resource import open_resource
 from aspen.simplates.pagination import split
+import pytest
 from pytest import raises
 
 
-# Tests
-# =====
+# Test outputs
 
 def test_barely_working(harness):
     output = harness.simple('Greetings, program!', 'index.html')
@@ -133,3 +138,48 @@ def test_offset_calculation_advanced(harness):
         '\n\n\n\n\n\n[---]\n'
         'Monkey\nHead\n') #Be careful: this is implicit concation, not a tuple
     check_offsets(raw, [0, 4, 6, 13])
+
+
+# Test the `open_resource` function
+
+def test_open_resource_opens_file_inside_www_root(harness):
+    harness.fs.www.mk(('index.html', 'foo'))
+    fspath = harness.fs.www.resolve('index.html')
+    with open_resource(harness.request_processor, fspath) as f:
+        actual = f.read()
+        assert actual == b'foo'
+
+def test_open_resource_opens_file_inside_project_root(harness):
+    harness.fs.project.mk(('error.spt', 'bar'))
+    spt_path = harness.fs.project.resolve('error.spt')
+    with open_resource(harness.request_processor, spt_path) as f:
+        actual = f.read()
+        assert actual == b'bar'
+
+# `realpath` doesn't work on Windows in Python < 3.8: https://bugs.python.org/issue9949
+@pytest.mark.xfail('os.path.realpath is os.path.abspath')
+def test_open_resource_raises_exception_when_file_is_outside(harness):
+    # Create a symlink, if possible.
+    executable = os.path.realpath(sys.executable)
+    fspath = harness.fs.www.resolve('python')
+    try:
+        if not hasattr(os, 'symlink'):
+            raise NotImplementedError
+        os.symlink(executable, fspath)
+        assert os.readlink(fspath) == executable
+        assert os.path.realpath(fspath) == executable
+    except NotImplementedError:
+        fspath = executable
+    # Initialize the request processor and check the warnings.
+    with catch_warnings(record=True) as messages:
+        harness.hydrate_request_processor()
+        if fspath != executable:
+            assert messages
+            for m in messages:
+                warning = m.message
+                assert isinstance(warning, PossibleBreakout)
+                assert warning.sym_path == fspath
+                assert warning.real_path == executable
+    # Attempt to open the resource.
+    with raises(AttemptedBreakout):
+        open_resource(harness.request_processor, fspath)