CVE-2012-5482: Permissions, Privileges, and Access Controls (CWE-264)
value: The v2 API in OpenStack Glance Grizzly, Folsom (2012.2), and Essex (2012.1) allows remote authenticated users to delete arbitrary non-protected images via an image deletion request.  NOTE: this vulnerability exists because of an incomplete fix for CVE-2012-4573.
CWE description: Weaknesses in this category are related to the management of permissions, privileges, and other security features that are used to perform access control.
Published: 2012-11-11T13:00Z
Repository: https://github.com/openstack/glance
Language: Python
Files changed: glance/api/v2/images.py, glance/tests/functional/v2/test_images.py
Commit date: 2012-11-08 10:56:07 -0800
Commit message: Ensure authorization before deleting from store

This fixes bug 1076506.

Change-Id: I3794c14fe523a9a27e943d73dd0248489d2b91f6

Diff: diff --git a/glance/api/v2/images.py b/glance/api/v2/images.py
index 04bd61bf..8443b43f 100644
--- a/glance/api/v2/images.py
+++ b/glance/api/v2/images.py
@@ -258,23 +258,26 @@ class ImagesController(object):
         if image['protected']:
             msg = _("Unable to delete as image %(image_id)s is protected"
                     % locals())
             raise webob.exc.HTTPForbidden(explanation=msg)
 
-        status = 'deleted'
-        if image['location']:
-            if CONF.delayed_delete:
-                status = 'pending_delete'
-                self.store_api.schedule_delayed_delete_from_backend(
-                                image['location'], id)
-            else:
-                self.store_api.safe_delete_from_backend(image['location'],
-                                                        req.context, id)
+        if image['location'] and CONF.delayed_delete:
+            status = 'pending_delete'
+        else:
+            status = 'deleted'
 
         try:
             self.db_api.image_update(req.context, image_id, {'status': status})
             self.db_api.image_destroy(req.context, image_id)
+
+            if image['location']:
+                if CONF.delayed_delete:
+                    self.store_api.schedule_delayed_delete_from_backend(
+                                    image['location'], id)
+                else:
+                    self.store_api.safe_delete_from_backend(image['location'],
+                                                            req.context, id)
         except (exception.NotFound, exception.Forbidden):
             msg = ("Failed to find image %(image_id)s to delete" % locals())
             LOG.info(msg)
             raise webob.exc.HTTPNotFound()
         else:
diff --git a/glance/tests/functional/v2/test_images.py b/glance/tests/functional/v2/test_images.py
index 186bc59f..0a6b4e83 100644
--- a/glance/tests/functional/v2/test_images.py
+++ b/glance/tests/functional/v2/test_images.py
@@ -216,10 +216,16 @@ class TestImages(functional.FunctionalTest):
         data = json.dumps({'name': 'image-1'})
         response = requests.post(path, headers=headers, data=data)
         self.assertEqual(201, response.status_code)
         image_id = json.loads(response.text)['id']
 
+        # Upload some image data
+        path = self._url('/v2/images/%s/file' % image_id)
+        headers = self._headers({'Content-Type': 'application/octet-stream'})
+        response = requests.put(path, headers=headers, data='ZZZZZ')
+        self.assertEqual(201, response.status_code)
+
         # TENANT1 should see the image in their list
         path = self._url('/v2/images')
         response = requests.get(path, headers=self._headers())
         self.assertEqual(200, response.status_code)
         images = json.loads(response.text)['images']
@@ -298,10 +304,16 @@ class TestImages(functional.FunctionalTest):
         path = self._url('/v2/images/%s' % image_id)
         headers = self._headers({'X-Tenant-Id': TENANT3})
         response = requests.delete(path, headers=headers)
         self.assertEqual(404, response.status_code)
 
+        # Image data should still be present after the failed delete
+        path = self._url('/v2/images/%s/file' % image_id)
+        response = requests.get(path, headers=self._headers())
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(response.text, 'ZZZZZ')
+
         self.stop_servers()
 
     def test_tag_lifecycle(self):
         # Create an image with a tag - duplicate should be ignored
         path = self._url('/v2/images')

Vulnerable code:         status = 'deleted'
        if image['location']:
            if CONF.delayed_delete:
                status = 'pending_delete'
                self.store_api.schedule_delayed_delete_from_backend(
                                image['location'], id)
            else:
                self.store_api.safe_delete_from_backend(image['location'],
                                                        req.context, id)
Fixed code:         if image['location'] and CONF.delayed_delete:
            status = 'pending_delete'
        else:
            status = 'deleted'

            if image['location']:
                if CONF.delayed_delete:
                    self.store_api.schedule_delayed_delete_from_backend(
                                    image['location'], id)
                else:
                    self.store_api.safe_delete_from_backend(image['location'],
                                                            req.context, id)
        # Upload some image data
        path = self._url('/v2/images/%s/file' % image_id)
        headers = self._headers({'Content-Type': 'application/octet-stream'})
        response = requests.put(path, headers=headers, data='ZZZZZ')
        self.assertEqual(201, response.status_code)

        # Image data should still be present after the failed delete
        path = self._url('/v2/images/%s/file' % image_id)
        response = requests.get(path, headers=self._headers())
        self.assertEqual(200, response.status_code)
        self.assertEqual(response.text, 'ZZZZZ')