Fix leak on error in v2i_POLICY_MAPPINGS If obj2 were invalid, obj1 leaks. Also both leak if creating the POLICY_MAPPINGS object fails on allocation error. Just swap the order, so the ASN1_OBJECTs go to an owned pointer from the start. Bug: oss-fuzz:55636 Change-Id: Ibf0bf58f44db510623035004f6eb1e00961a5454 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56805 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: Bob Beck <bbe@google.com> Reviewed-by: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> Commit-Queue: Adam Langley <agl@google.com>
diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc index cd231c8..16c47d4 100644 --- a/crypto/x509/x509_test.cc +++ b/crypto/x509/x509_test.cc
@@ -5762,6 +5762,14 @@ {"subjectAltName", "otherName:invalid_oid;BOOLEAN:TRUE", nullptr, {}}, {"subjectAltName", "otherName:1.2.3.4;invalid_value", nullptr, {}}, + {"policyMappings", + "1.1.1.1:2.2.2.2", + nullptr, + {0x30, 0x15, 0x06, 0x03, 0x55, 0x1d, 0x21, 0x04, 0x0e, 0x30, 0x0c, 0x30, + 0x0a, 0x06, 0x03, 0x29, 0x01, 0x01, 0x06, 0x03, 0x52, 0x02, 0x02}}, + {"policyMappings", "invalid_oid:2.2.2.2", nullptr, {}}, + {"policyMappings", "1.1.1.1:invalid_oid", nullptr, {}}, + // The "DER:" prefix just specifies an arbitrary byte string. Colons // separators are ignored. {kTestOID, "DER:0001020304", nullptr, {0x30, 0x15, 0x06, 0x0c, 0x2a, 0x86,
diff --git a/crypto/x509v3/v3_pmaps.c b/crypto/x509v3/v3_pmaps.c index b6f3384..dae4b66 100644 --- a/crypto/x509v3/v3_pmaps.c +++ b/crypto/x509v3/v3_pmaps.c
@@ -124,28 +124,29 @@ for (size_t i = 0; i < sk_CONF_VALUE_num(nval); i++) { const CONF_VALUE *val = sk_CONF_VALUE_value(nval, i); if (!val->value || !val->name) { - sk_POLICY_MAPPING_pop_free(pmaps, POLICY_MAPPING_free); OPENSSL_PUT_ERROR(X509V3, X509V3_R_INVALID_OBJECT_IDENTIFIER); X509V3_conf_err(val); - return NULL; + goto err; } - ASN1_OBJECT *obj1 = OBJ_txt2obj(val->name, 0); - ASN1_OBJECT *obj2 = OBJ_txt2obj(val->value, 0); - if (!obj1 || !obj2) { - sk_POLICY_MAPPING_pop_free(pmaps, POLICY_MAPPING_free); - OPENSSL_PUT_ERROR(X509V3, X509V3_R_INVALID_OBJECT_IDENTIFIER); - X509V3_conf_err(val); - return NULL; - } + POLICY_MAPPING *pmap = POLICY_MAPPING_new(); if (pmap == NULL || !sk_POLICY_MAPPING_push(pmaps, pmap)) { POLICY_MAPPING_free(pmap); - sk_POLICY_MAPPING_pop_free(pmaps, POLICY_MAPPING_free); OPENSSL_PUT_ERROR(X509V3, ERR_R_MALLOC_FAILURE); - return NULL; + goto err; } - pmap->issuerDomainPolicy = obj1; - pmap->subjectDomainPolicy = obj2; + + pmap->issuerDomainPolicy = OBJ_txt2obj(val->name, 0); + pmap->subjectDomainPolicy = OBJ_txt2obj(val->value, 0); + if (!pmap->issuerDomainPolicy || !pmap->subjectDomainPolicy) { + OPENSSL_PUT_ERROR(X509V3, X509V3_R_INVALID_OBJECT_IDENTIFIER); + X509V3_conf_err(val); + goto err; + } } return pmaps; + +err: + sk_POLICY_MAPPING_pop_free(pmaps, POLICY_MAPPING_free); + return NULL; }