Fix double free in policy check code (CVE-2011-4109)
authorDr. Stephen Henson <steve@openssl.org>
Wed, 4 Jan 2012 19:00:28 +0000 (19:00 +0000)
committerDr. Stephen Henson <steve@openssl.org>
Wed, 4 Jan 2012 19:00:28 +0000 (19:00 +0000)
CHANGES
crypto/x509v3/pcy_map.c
crypto/x509v3/pcy_tree.c

diff --git a/CHANGES b/CHANGES
index ae0b41c84396a59fc3dc4eec4efbf141f8fe14af..cf32f605eb0d9eae17d5b1a3d9b2a35a88468324 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,9 @@
 
  Changes between 0.9.8r and 0.9.8s [xx XXX xxxx]
 
+  *) Stop policy check failure freeing same buffer twice. (CVE-2011-4109)
+     [Ben Laurie, Kasper <ekasper@google.com>]
+
   *) Clear bytes used for block padding of SSL 3.0 records.
      (CVE-2011-4576)
      [Adam Langley (Google)]
index f28796e6d4eba35e6dea299c5724a05a736858f5..acd2ede6f34c0d4fac4ec5173b0d7b41f82ca981 100644 (file)
@@ -70,8 +70,6 @@ static int ref_cmp(const X509_POLICY_REF * const *a,
 
 static void policy_map_free(X509_POLICY_REF *map)
        {
-       if (map->subjectDomainPolicy)
-               ASN1_OBJECT_free(map->subjectDomainPolicy);
        OPENSSL_free(map);
        }
 
@@ -95,6 +93,7 @@ int policy_cache_set_mapping(X509 *x, POLICY_MAPPINGS *maps)
        {
        POLICY_MAPPING *map;
        X509_POLICY_REF *ref = NULL;
+       ASN1_OBJECT *subjectDomainPolicyRef;
        X509_POLICY_DATA *data;
        X509_POLICY_CACHE *cache = x->policy_cache;
        int i;
@@ -153,13 +152,16 @@ int policy_cache_set_mapping(X509 *x, POLICY_MAPPINGS *maps)
                if (!sk_ASN1_OBJECT_push(data->expected_policy_set, 
                                                map->subjectDomainPolicy))
                        goto bad_mapping;
+                /* map->subjectDomainPolicy will be freed when
+                 * cache->data is freed. Set it to NULL to avoid double-free. */
+                subjectDomainPolicyRef = map->subjectDomainPolicy;
+                map->subjectDomainPolicy = NULL;
                
                ref = OPENSSL_malloc(sizeof(X509_POLICY_REF));
                if (!ref)
                        goto bad_mapping;
 
-               ref->subjectDomainPolicy = map->subjectDomainPolicy;
-               map->subjectDomainPolicy = NULL;
+               ref->subjectDomainPolicy = subjectDomainPolicyRef;
                ref->data = data;
 
                if (!sk_X509_POLICY_REF_push(cache->maps, ref))
index 89f84bfa18b2edff4eeb7e9a4a8cb0be7de7c04d..92ad0a2b399176c8a609a9ece788c8cb39fb6a13 100644 (file)
@@ -612,6 +612,10 @@ int X509_policy_check(X509_POLICY_TREE **ptree, int *pexplicit_policy,
                case 2:
                return 1;
 
+                /* Some internal error */
+               case -1:
+               return -1;
+
                /* Some internal error */
                case 0:
                return 0;
@@ -691,4 +695,3 @@ int X509_policy_check(X509_POLICY_TREE **ptree, int *pexplicit_policy,
        return 0;
 
        }
-