CVE-2011-4596: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: Multiple directory traversal vulnerabilities in OpenStack Nova before 2011.3.1, when the EC2 API and the S3/RegisterImage image-registration method are enabled, allow remote authenticated users to overwrite arbitrary files via a crafted (1) tarball or (2) manifest.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
Published: 2011-12-23T22:55Z
Repository: https://github.com/openstack/nova
Language: Python
Files changed: MANIFEST.in, nova/image/s3.py, nova/tests/image/test_s3.py
Commit date: 2011-12-13 16:00:41 +0100
Commit message: 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

Diff: diff --git a/MANIFEST.in b/MANIFEST.in
index b10dafc7b..2a947f823 100644
--- a/MANIFEST.in
+++ b/MANIFEST.in
@@ -35,9 +35,10 @@ include nova/tests/bundle/1mb.manifest.xml
 include nova/tests/bundle/1mb.no_kernel_or_ramdisk.manifest.xml
 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
 global-exclude *.pyc
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
@@ -153,11 +153,11 @@ class S3ImageService(object):
                                                host=FLAGS.s3_host)
 
     @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
 
     def _s3_parse_manifest(self, context, metadata, manifest):
         manifest = ElementTree.fromstring(manifest)
@@ -385,12 +385,23 @@ class S3ImageService(object):
             raise exception.Error(_('Failed to decrypt image file '
                                     '%(image_file)s: %(err)s') %
                                     {'image_file': encrypted_filename,
                                      '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]
         tar_file.close()
         return os.path.join(path, image_file)
diff --git a/nova/tests/image/abs.tar.gz b/nova/tests/image/abs.tar.gz
new file mode 100644
index 000000000..4d3950734
Binary files /dev/null and b/nova/tests/image/abs.tar.gz differ
diff --git a/nova/tests/image/rel.tar.gz b/nova/tests/image/rel.tar.gz
new file mode 100644
index 000000000..b54f55aa7
Binary files /dev/null and b/nova/tests/image/rel.tar.gz differ
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
@@ -13,10 +13,12 @@
 #    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    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
 from nova import test
 from nova.image import s3
@@ -128,5 +130,13 @@ class TestS3ImageService(test.TestCase):
             {'device_name': '/dev/sda2',
              'virutal_name': 'ephemeral0'},
             {'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'))

Vulnerable code:         local_filename = os.path.join(local_dir, filename)
Fixed code: include nova/tests/image/*.tar.gz
        local_filename = os.path.join(local_dir, os.path.basename(filename))
    @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()

        S3ImageService._test_for_malicious_tarball(path, filename)
import os


    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'))