Sanitize EC2 manifests and image tarballs

Prevent potential directory traversal with malicious EC2 image tarballs,
by making sure the tarfile is safe before unpacking it. Fixes bug 894755

Prevent potential directory traversal with malicious file names in
EC2 image manifests. Fixes bug 885167

Change-Id: If6109047307bd6e654ee9d1254f0d7f31cf741c1

5 files changed, 23 insertions, 1 deletions

diff --git a/MANIFEST.in b/MANIFEST.in
index b10dafc7b..2a947f823 100644
--- a/MANIFEST.in
+++ b/MANIFEST.in
@@ -37,6 +37,7 @@ include nova/tests/bundle/1mb.part.0
 include nova/tests/bundle/1mb.part.1
 include nova/tests/api/ec2/public_key/*
 include nova/tests/db/nova.austin.sqlite
+include nova/tests/image/*.tar.gz
 include plugins/xenapi/README
 include plugins/xenapi/etc/xapi.d/plugins/objectstore
 include plugins/xenapi/etc/xapi.d/plugins/pluginlib_nova.py
diff --git a/nova/image/s3.py b/nova/image/s3.py
index ea0882d39..46a940a69 100644
--- a/nova/image/s3.py
+++ b/nova/image/s3.py
@@ -155,7 +155,7 @@ class S3ImageService(object):
     @staticmethod
     def _download_file(bucket, filename, local_dir):
         key = bucket.get_key(filename)
-        local_filename = os.path.join(local_dir, filename)
+        local_filename = os.path.join(local_dir, os.path.basename(filename))
         key.get_contents_to_filename(local_filename)
         return local_filename
 
@@ -388,7 +388,18 @@ class S3ImageService(object):
                                      'err': err})
 
     @staticmethod
+    def _test_for_malicious_tarball(path, filename):
+        """Raises exception if extracting tarball would escape extract path"""
+        tar_file = tarfile.open(filename, 'r|gz')
+        for n in tar_file.getnames():
+            if not os.path.abspath(os.path.join(path, n)).startswith(path):
+                tar_file.close()
+                raise exception.Error(_('Unsafe filenames in image'))
+        tar_file.close()
+
+    @staticmethod
     def _untarzip_image(path, filename):
+        S3ImageService._test_for_malicious_tarball(path, filename)
         tar_file = tarfile.open(filename, 'r|gz')
         tar_file.extractall(path)
         image_file = tar_file.getnames()[0]
diff --git a/nova/tests/image/abs.tar.gz b/nova/tests/image/abs.tar.gz
new file mode 100644
index 000000000..4d3950734
--- /dev/null
+++ b/nova/tests/image/abs.tar.gz
diff --git a/nova/tests/image/rel.tar.gz b/nova/tests/image/rel.tar.gz
new file mode 100644
index 000000000..b54f55aa7
--- /dev/null
+++ b/nova/tests/image/rel.tar.gz
diff --git a/nova/tests/image/test_s3.py b/nova/tests/image/test_s3.py
index 02f66fce1..2a9d279f4 100644
--- a/nova/tests/image/test_s3.py
+++ b/nova/tests/image/test_s3.py
@@ -15,6 +15,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import os
+
 from nova import context
 import nova.db.api
 from nova import exception
@@ -130,3 +132,11 @@ class TestS3ImageService(test.TestCase):
             {'device_name': '/dev/sdb0',
              'no_device': True}]
         self.assertEqual(block_device_mapping, expected_bdm)
+
+    def test_s3_malicious_tarballs(self):
+        self.assertRaises(exception.Error,
+            self.image_service._test_for_malicious_tarball,
+            "/unused", os.path.join(os.path.dirname(__file__), 'abs.tar.gz'))
+        self.assertRaises(exception.Error,
+            self.image_service._test_for_malicious_tarball,
+            "/unused", os.path.join(os.path.dirname(__file__), 'rel.tar.gz'))