Explorar o código

[filebrowser] Fix archive upload directory issue

krish %!s(int64=11) %!d(string=hai) anos
pai
achega
9050f1b9db

+ 18 - 3
apps/filebrowser/src/filebrowser/lib/archives.py

@@ -21,11 +21,11 @@ import os
 import posixpath
 import tarfile
 import tempfile
-from zipfile import ZipFile
-
-from filebrowser.conf import ARCHIVE_UPLOAD_TEMPDIR
 
+from desktop.lib.exceptions_renderable import PopupException
 from django.utils.translation import ugettext as _
+from filebrowser.conf import ARCHIVE_UPLOAD_TEMPDIR
+from zipfile import ZipFile
 
 
 __all__ = ['archive_factory']
@@ -47,6 +47,10 @@ class Archive(object):
     Creates all directories passed at the given basepath.
     """
     for directory in dirs:
+      # Stops if directory start with '/' or points to a relative path
+      if os.path.isabs(directory) or '..' in directory:
+        raise IllegalPathException()
+
       directory = os.path.join(basepath, directory)
       try:
         os.makedirs(directory)
@@ -147,6 +151,12 @@ class TarballArchive(Archive):
         dirs.append(tarinfo.name)
       else:
         files.append(tarinfo.name)
+        parent = os.path.dirname(tarinfo.path)
+        # getmembers() sometimes doesn't return all the directories
+        # Go up the path one directory at the time
+        while parent != '' and parent not in dirs:
+          dirs.append(parent)
+          parent = os.path.dirname(parent)
     return (dirs, files)
 
   def _create_files(self, basepath, files=[]):
@@ -166,3 +176,8 @@ def archive_factory(path, archive_type='zip'):
     return ZipArchive(path)
   elif archive_type == 'tarball' or archive_type == 'tar.gz' or archive_type == 'tgz':
     return TarballArchive(path)
+
+class IllegalPathException(PopupException):
+
+  def __init__(self):
+    super(IllegalPathException, self).__init__('''Archive path cannot be absolute or contain '..' ''')

+ 23 - 3
apps/filebrowser/src/filebrowser/lib/archives_test.py

@@ -15,14 +15,13 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import archives
 import unittest
 import os
 
+from archives import IllegalPathException
 from nose.tools import assert_true, assert_equal
 
-import archives
-
-
 class ArchiveTest(unittest.TestCase):
 
   def test_zip(self):
@@ -47,6 +46,27 @@ class ArchiveTest(unittest.TestCase):
     assert_true(os.path.isfile(directory + '/test.txt'))
     assert_equal(os.path.getsize(directory + '/test.txt'), 4)
 
+    FILE = os.path.realpath('apps/filebrowser/src/filebrowser/test_data/test2.tar.gz')
+
+    # Extract the file
+    # This file should only have 'test.txt' in it
+    directory = archives.archive_factory(FILE, 'tar.gz').extract()
+    assert_true(os.path.exists(directory))
+    assert_true(os.path.isdir(directory))
+    assert_true(os.path.isfile(directory + '/home/docs/test.txt'))
+    assert_equal(os.path.getsize(directory + '/home/docs/test.txt'), 4)
+
+    # This file should not be extracted as it contains illegal path '../../../Desktop/test.txt'
+    FILE = os.path.realpath('apps/filebrowser/src/filebrowser/test_data/test3.tar.gz')
+
+    factory = archives.archive_factory(FILE, 'tar.gz')
+    self.assertRaises(IllegalPathException, factory.extract)
+
+    # This file should not be extracted as it contains absolute path
+    FILE = os.path.realpath('apps/filebrowser/src/filebrowser/test_data/test4.tar.gz')
+
+    factory = archives.archive_factory(FILE, 'tar.gz')
+    self.assertRaises(IllegalPathException, factory.extract)
 
 if __name__ == "__main__":
   unittest.main()

BIN=BIN
apps/filebrowser/src/filebrowser/test_data/test2.tar.gz


BIN=BIN
apps/filebrowser/src/filebrowser/test_data/test3.tar.gz


BIN=BIN
apps/filebrowser/src/filebrowser/test_data/test4.tar.gz