Fix memory leaks in X509 certificate parsing/verification.

This commit is contained in:
Jon Siwek 2014-05-06 20:50:37 -05:00
parent 37b860d325
commit 6277be6e60
2 changed files with 20 additions and 9 deletions

View file

@ -153,6 +153,8 @@ RecordVal* file_analysis::X509::ParseCertificate(X509Val* cert_val)
unsigned int length = KeyLength(pkey); unsigned int length = KeyLength(pkey);
if ( length > 0 ) if ( length > 0 )
pX509Cert->Assign(9, new Val(length, TYPE_COUNT)); pX509Cert->Assign(9, new Val(length, TYPE_COUNT));
EVP_PKEY_free(pkey);
} }
@ -273,6 +275,7 @@ void file_analysis::X509::ParseBasicConstraints(X509_EXTENSION* ex)
vl->append(pBasicConstraint); vl->append(pBasicConstraint);
mgr.QueueEvent(x509_ext_basic_constraints, vl); mgr.QueueEvent(x509_ext_basic_constraints, vl);
BASIC_CONSTRAINTS_free(constr);
} }
else else
@ -387,6 +390,7 @@ void file_analysis::X509::ParseSAN(X509_EXTENSION* ext)
vl->append(GetFile()->GetVal()->Ref()); vl->append(GetFile()->GetVal()->Ref());
vl->append(sanExt); vl->append(sanExt);
mgr.QueueEvent(x509_ext_subject_alternative_name, vl); mgr.QueueEvent(x509_ext_subject_alternative_name, vl);
GENERAL_NAMES_free(altname);
} }
StringVal* file_analysis::X509::KeyCurve(EVP_PKEY *key) StringVal* file_analysis::X509::KeyCurve(EVP_PKEY *key)
@ -442,13 +446,20 @@ unsigned int file_analysis::X509::KeyLength(EVP_PKEY *key)
return 0; return 0;
const EC_GROUP *group = EC_KEY_get0_group(key->pkey.ec); const EC_GROUP *group = EC_KEY_get0_group(key->pkey.ec);
if ( ! group ) if ( ! group )
{
// unknown ex-group // unknown ex-group
BN_free(ec_order);
return 0; return 0;
}
if ( ! EC_GROUP_get_order(group, ec_order, NULL) ) if ( ! EC_GROUP_get_order(group, ec_order, NULL) )
{
// could not get ec-group-order // could not get ec-group-order
BN_free(ec_order);
return 0; return 0;
}
unsigned int length = BN_num_bits(ec_order); unsigned int length = BN_num_bits(ec_order);
BN_free(ec_order); BN_free(ec_order);

View file

@ -179,7 +179,7 @@ function x509_verify%(certs: x509_opaque_vector, root_certs: table_string_of_str
X509* x = ((file_analysis::X509Val*) sv)->GetCertificate(); X509* x = ((file_analysis::X509Val*) sv)->GetCertificate();
if ( ! x ) if ( ! x )
{ {
sk_X509_pop(untrusted_certs); sk_X509_free(untrusted_certs);
builtin_error(fmt("No certificate in opaque in stack")); builtin_error(fmt("No certificate in opaque in stack"));
return x509_error_record(-1, "No certificate in opaque"); return x509_error_record(-1, "No certificate in opaque");
} }
@ -203,6 +203,7 @@ function x509_verify%(certs: x509_opaque_vector, root_certs: table_string_of_str
if ( ! chain ) if ( ! chain )
{ {
reporter->Error("Encountered valid chain that could not be resolved"); reporter->Error("Encountered valid chain that could not be resolved");
sk_X509_pop_free(chain, X509_free);
goto x509_verify_chainerror; goto x509_verify_chainerror;
} }
@ -212,22 +213,21 @@ function x509_verify%(certs: x509_opaque_vector, root_certs: table_string_of_str
for ( int i = 0; i < num_certs; i++ ) for ( int i = 0; i < num_certs; i++ )
{ {
X509* currcert = sk_X509_value(chain, i); X509* currcert = sk_X509_value(chain, i);
if ( !currcert )
{
reporter->InternalError("OpenSSL returned null certificate");
goto x509_verify_chainerror;
}
chainVector->Assign(i, new file_analysis::X509Val(currcert)); // X509Val takes ownership if ( currcert )
chainVector->Assign(i, new file_analysis::X509Val(currcert)); // X509Val takes ownership
else
reporter->InternalWarning("OpenSSL returned null certificate");
} }
sk_X509_free(chain);
} }
x509_verify_chainerror: x509_verify_chainerror:
X509_STORE_CTX_cleanup(&csc); X509_STORE_CTX_cleanup(&csc);
if ( untrusted_certs ) sk_X509_free(untrusted_certs);
sk_X509_pop(untrusted_certs);
RecordVal* rrecord = new RecordVal(BifType::Record::X509::Result); RecordVal* rrecord = new RecordVal(BifType::Record::X509::Result);