Merge pull request #1357 from chenl/master

fix issue#1261: security issue: gluon.tools.Expose will follow symlinks

gluon/tests/test_tools.py

@@ -6,6 +6,8 @@
 """
 import os
 import sys
+import shutil
+import tempfile
 import smtplib
 import datetime
 import unittest
@@ -18,11 +20,12 @@ DEFAULT_URI = os.getenv('DB', 'sqlite:memory')
 
 from gluon.dal import DAL, Field
 from pydal.objects import Table
-from tools import Auth, Mail, Recaptcha, Recaptcha2, prettydate
+import tools
+from tools import Auth, Mail, Recaptcha, Recaptcha2, prettydate, Expose
 from gluon.globals import Request, Response, Session
-from storage import Storage
 from languages import translator
 from gluon.http import HTTP
+from gluon import SPAN, H3, TABLE, TR, TD, A, URL, current
 
 python_version = sys.version[:3]
 IS_IMAP = "imap" in DEFAULT_URI
@@ -989,9 +992,6 @@ class TestAuth(unittest.TestCase):
 # TODO: class TestPluginManager(unittest.TestCase):
 
 
-# TODO: class TestExpose(unittest.TestCase):
-
-
 # TODO: class TestWiki(unittest.TestCase):
 
 
@@ -1081,5 +1081,229 @@ class TestToolsFunctions(unittest.TestCase):
         self.assertEqual(prettydate(d='invalid_date'), '[invalid date]')
 
 
+pjoin = os.path.join
+
+def have_symlinks():
+    return os.name == 'posix'
+
+
+class Test_Expose__in_base(unittest.TestCase):
+
+    def test_in_base(self):
+        are_under = [ # (sub, base)
+            ('/foo/bar', '/foo'),
+            ('/foo', '/foo'),
+            ('/foo', '/'),
+            ('/', '/'),
+        ]
+        for sub, base in are_under:
+            self.assertTrue( Expose._Expose__in_base(subdir=sub, basedir=base, sep='/'),
+                             '%s is not under %s' % (sub, base) )
+
+    def test_not_in_base(self):
+        are_not_under = [ # (sub, base)
+            ('/foobar', '/foo'),
+            ('/foo', '/foo/bar'),
+            ('/bar', '/foo'),
+            ('/foo/bar', '/bar'),
+            ('/', '/x'),
+        ]
+        for sub, base in are_not_under:
+            self.assertFalse( Expose._Expose__in_base(subdir=sub, basedir=base, sep='/'),
+                              '%s should not be under %s' % (sub, base) )
+
+
+class TestExpose(unittest.TestCase):
+
+    def setUp(self):
+        self.base_dir = tempfile.mkdtemp()
+
+        self.make_dirs()
+        self.touch_files()
+        self.make_readme()
+        if have_symlinks():
+            self.make_symlinks()
+
+        # $BASE/
+        # |-- inside/
+        # |   |-- dir1/
+        # |   |   |-- file1
+        # |   |   `-- file2
+        # |   |-- dir2/
+        # |   |   |-- link_to_dir1/@ -> $BASE/inside/dir1/
+        # |   |   `-- link_to_file1@ -> $BASE/inside/dir1/file1
+        # |   |-- link_to_outside/@ -> $BASE/outside/
+        # |   |-- link_to_file3@ -> $BASE/outside/file3
+        # |   `-- README
+        # `-- outside/
+        #     `-- file3
+
+        self.set_expectations()
+        tools.URL = lambda args: URL(a='a', c='c', f='f', args=args)
+
+    def tearDown(self):
+        tools.URL = URL
+        shutil.rmtree(self.base_dir)
+
+    def make_dirs(self):
+        """setup direcotry strucutre"""
+        for d in (['inside'],
+                  ['inside', 'dir1'],
+                  ['inside', 'dir2'],
+                  ['outside']):
+            os.mkdir(pjoin(self.base_dir, *d))
+
+    def touch_files(self):
+        """create some files"""
+        for f in (['inside', 'dir1', 'file1'],
+                  ['inside', 'dir1', 'file2'],
+                  ['outside', 'file3']):
+            with open(pjoin(self.base_dir, *f), 'a'):
+                pass
+
+    def make_readme(self):
+        with open(pjoin(self.base_dir, 'inside', 'README'), 'w') as f:
+            f.write('README content')
+
+    def make_symlinks(self):
+        """setup extenstion for posix systems"""
+        # inside links
+        os.symlink(
+            pjoin(self.base_dir, 'inside', 'dir1'),
+            pjoin(self.base_dir, 'inside', 'dir2', 'link_to_dir1'))
+        os.symlink(
+            pjoin(self.base_dir, 'inside', 'dir1', 'file1'),
+            pjoin(self.base_dir, 'inside', 'dir2', 'link_to_file1'))
+        # outside links
+        os.symlink(
+            pjoin(self.base_dir, 'outside'),
+            pjoin(self.base_dir, 'inside', 'link_to_outside'))
+        os.symlink(
+            pjoin(self.base_dir, 'outside', 'file3'),
+            pjoin(self.base_dir, 'inside', 'link_to_file3'))
+
+    def set_expectations(self):
+        url = lambda args: URL('a', 'c', 'f', args=args)
+
+        self.expected_folders = {}
+        self.expected_folders['inside'] = SPAN(H3('Folders'), TABLE(
+            TR(TD(A('dir1', _href=url(args=['dir1'])))),
+            TR(TD(A('dir2', _href=url(args=['dir2'])))),
+            _class='table',
+        ))
+        self.expected_folders[pjoin('inside', 'dir1')] = ''
+        if have_symlinks():
+            self.expected_folders[pjoin('inside', 'dir2')] = SPAN(H3('Folders'), TABLE(
+                TR(TD(A('link_to_dir1', _href=url(args=['dir2', 'link_to_dir1'])))),
+                _class='table',
+            ))
+        else:
+            self.expected_folders[pjoin('inside', 'dir2')] = ''
+
+        self.expected_files = {}
+        self.expected_files['inside'] = SPAN(H3('Files'), TABLE(
+            TR(TD(A('README', _href=url(args=['README']))), TD('')),
+            _class='table',
+        ))
+        self.expected_files[pjoin('inside', 'dir1')] = SPAN(H3('Files'), TABLE(
+            TR(TD(A('file1', _href=url(args=['dir1', 'file1']))), TD('')),
+            TR(TD(A('file2', _href=url(args=['dir1', 'file2']))), TD('')),
+            _class='table',
+        ))
+        if have_symlinks():
+            self.expected_files[pjoin('inside', 'dir2')] = SPAN(H3('Files'), TABLE(
+                TR(TD(A('link_to_file1', _href=url(args=['dir2', 'link_to_file1']))), TD('')),
+                _class='table',
+            ))
+        else:
+            self.expected_files[pjoin('inside', 'dir2')] = ''
+
+    def make_expose(self, base, show='', follow_symlink_out=False):
+        current.request = Request(env={})
+        current.request.raw_args = show
+        current.request.args = show.split('/')
+        return Expose(base=pjoin(self.base_dir, base),
+                      basename=base,
+                      follow_symlink_out=follow_symlink_out)
+
+    def test_expose_inside_state(self):
+        expose = self.make_expose(base='inside', show='')
+        self.assertEqual(expose.args, [])
+        self.assertEqual(expose.folders, ['dir1', 'dir2'])
+        self.assertEqual(expose.filenames, ['README'])
+
+    @unittest.skipUnless(have_symlinks(), 'requires symlinks')
+    def test_expose_inside_state_floow_symlink_out(self):
+        expose = self.make_expose(base='inside', show='',
+                                  follow_symlink_out=True)
+        self.assertEqual(expose.args, [])
+        self.assertEqual(expose.folders, ['dir1', 'dir2', 'link_to_outside'])
+        self.assertEqual(expose.filenames, ['README', 'link_to_file3'])
+
+    def test_expose_inside_dir1_state(self):
+        expose = self.make_expose(base='inside', show='dir1')
+        self.assertEqual(expose.args, ['dir1'])
+        self.assertEqual(expose.folders, [])
+        self.assertEqual(expose.filenames, ['file1', 'file2'])
+
+    def test_expose_inside_dir2_state(self):
+        expose = self.make_expose(base='inside', show='dir2')
+        self.assertEqual(expose.args, ['dir2'])
+        if have_symlinks():
+            self.assertEqual(expose.folders, ['link_to_dir1'])
+            self.assertEqual(expose.filenames, ['link_to_file1'])
+        else:
+            self.assertEqual(expose.folders, [])
+            self.assertEqual(expose.filenames, [])
+
+    def test_expose_base_inside_state(self):
+        expose = self.make_expose(base='', show='inside')
+        self.assertEqual(expose.args, ['inside'])
+        if have_symlinks():
+            self.assertEqual(expose.folders, ['dir1', 'dir2', 'link_to_outside'])
+            self.assertEqual(expose.filenames, ['README', 'link_to_file3'])
+        else:
+            self.assertEqual(expose.folders, ['dir1', 'dir2'])
+            self.assertEqual(expose.filenames, ['README'])
+
+    def test_expose_base_inside_dir2_state(self):
+        expose = self.make_expose(base='', show='inside/dir2')
+        self.assertEqual(expose.args, ['inside', 'dir2'])
+        if have_symlinks():
+            self.assertEqual(expose.folders, ['link_to_dir1'])
+            self.assertEqual(expose.filenames, ['link_to_file1'])
+        else:
+            self.assertEqual(expose.folders, [])
+            self.assertEqual(expose.filenames, [])
+
+    def assertSameXML(self, a, b):
+        self.assertEqual(a if isinstance(a, str) else a.xml(),
+                         b if isinstance(b, str) else b.xml())
+
+    def run_test_xml_for(self, base, show):
+        expose = self.make_expose(base, show)
+        path = pjoin(base, show).rstrip(os.path.sep)
+        request = Request(env={})
+        self.assertSameXML(expose.table_files(), self.expected_files[path])
+        self.assertSameXML(expose.table_folders(), self.expected_folders[path])
+
+    def test_xml_inside(self):
+        self.run_test_xml_for(base='inside', show='')
+
+    def test_xml_dir1(self):
+        self.run_test_xml_for(base='inside', show='dir1')
+
+    def test_xml_dir2(self):
+        self.run_test_xml_for(base='inside', show='dir2')
+
+    def test_file_not_found(self):
+        with self.assertRaises(HTTP):
+            self.make_expose(base='inside', show='dir1/file_not_found')
+
+    def test_not_authorized(self):
+        with self.assertRaises(HTTP):
+            self.make_expose(base='inside', show='link_to_file3')
+
+
 if __name__ == '__main__':
     unittest.main()

gluon/tools.py

@@ -6212,7 +6212,8 @@ class PluginManager(object):
 
 
 class Expose(object):
-    def __init__(self, base=None, basename=None, extensions=None, allow_download=True):
+    def __init__(self, base=None, basename=None, extensions=None,
+                 allow_download=True, follow_symlink_out=False):
         """
         Examples:
             Use as::
@@ -6230,10 +6231,19 @@ class Expose(object):
             extensions: an optional list of file extensions for filtering
                 displayed files: e.g. `['.py', '.jpg']`
             allow_download: whether to allow downloading selected files
+            follow_symlink_out: whether to follow symbolic links that points
+                points outside of `base`.
+                Warning: setting this to `True` might pose a security risk
+                         if you don't also have complete control over writing
+                         and file creation under `base`.
 
         """
         current.session.forget()
-        base = base or os.path.join(current.request.folder, 'static')
+        self.follow_symlink_out = follow_symlink_out
+        self.base = self.normalize_path(
+            base or os.path.join(current.request.folder, 'static'))
+        self.basename = basename or current.request.function
+        self.base = base = os.path.realpath(base or os.path.join(current.request.folder, 'static'))
         basename = basename or current.request.function
         self.basename = basename
 
@@ -6241,19 +6251,23 @@ class Expose(object):
             self.args = [arg for arg in current.request.raw_args.split('/') if arg]
         else:
             self.args = [arg for arg in current.request.args if arg]
-        filename = os.path.join(base, *self.args)
+
+        filename = os.path.join(self.base, *self.args)
         if not os.path.exists(filename):
             raise HTTP(404, "FILE NOT FOUND")
-        if not os.path.normpath(filename).startswith(base):
+        if not self.in_base(filename):
             raise HTTP(401, "NOT AUTHORIZED")
         if allow_download and not os.path.isdir(filename):
             current.response.headers['Content-Type'] = contenttype(filename)
             raise HTTP(200, open(filename, 'rb'), **current.response.headers)
         self.path = path = os.path.join(filename, '*')
-        self.folders = [f[len(path) - 1:] for f in sorted(glob.glob(path))
+        dirname_len = len(path) - 1
-                        if os.path.isdir(f) and not self.isprivate(f)]
+        allowed = [f for f in sorted(glob.glob(path))
-        self.filenames = [f[len(path) - 1:] for f in sorted(glob.glob(path))
+                   if not any([self.isprivate(f), self.issymlink_out(f)])]
-                          if not os.path.isdir(f) and not self.isprivate(f)]
+        self.folders = [f[dirname_len:]
+                        for f in allowed if os.path.isdir(f)]
+        self.filenames = [f[dirname_len:]
+                          for f in allowed if not os.path.isdir(f)]
         if 'README' in self.filenames:
             readme = open(os.path.join(filename, 'README')).read()
             self.paragraph = MARKMIN(readme)
@@ -6280,6 +6294,31 @@ class Expose(object):
                                 for folder in self.folders], **dict(_class="table")))
         return ''
 
+    @staticmethod
+    def __in_base(subdir, basedir, sep=os.path.sep):
+        """True if subdir/ is under basedir/"""
+        s = lambda f: '%s%s' % (f.rstrip(sep), sep) # f -> f/
+        # The trailing '/' is for the case of '/foobar' in_base of '/foo':
+        # - becase '/foobar'  starts with        '/foo'
+        # - but    '/foobar/' doesn't start with '/foo/'
+        return s(subdir).startswith(s(basedir))
+
+    def in_base(self, f):
+        """True if f/ is under self.base/
+        Where f ans slef.base are normalized paths
+        """
+        return self.__in_base(self.normalize_path(f), self.base)
+
+    def normalize_path(self, f):
+        if self.follow_symlink_out:
+            return os.path.normpath(f)
+        else:
+            return os.path.realpath(f)
+
+    def issymlink_out(self, f):
+        "True if f is a symlink and is pointing outside of self.base"
+        return os.path.islink(f) and not self.in_base(f)
+
     @staticmethod
     def isprivate(f):
         return 'private' in f or f.startswith('.') or f.endswith('~')