diff options
author | Max Bires <jbires@google.com> | 2021-06-27 19:32:40 -0700 |
---|---|---|
committer | Max Bires <jbires@google.com> | 2021-06-28 01:46:16 -0700 |
commit | f9626909bc0d5324b06f2febe7369d1761d00be7 (patch) | |
tree | be58802ddd1203d711b9254537c33fa697833e33 | |
parent | 6e061db49f3565cfbc7c81bea8fe1b8aa27b75e1 (diff) | |
download | keymaster-f9626909bc0d5324b06f2febe7369d1761d00be7.tar.gz |
Fixing passing pointer to uninitialized object.
There was a bug in the code for extracting the raw EC public key x and y
coordinates from the certificate retrieved from keymaster. Two boringssl
BIGNUM objects were declared on the stack with pointers passed in
directly to a function without the objects first being initialized. This
went undetected in Cuttlefish, but predictably caused KM on Trusty to
attempt to access a memory page it didn't own.
This patch also fixes a pair of memory leaks and cleans up the memory
management.
Ignore-AOSP-First: Cherry-pick from AOSP
Bug: 192199094
Test: atest VtsHalRemotelyProvisionedComponentTargetTests
Change-Id: Ic513dc9fc2146ef0c131f04088f1d25c9963bc2a
-rw-r--r-- | km_openssl/openssl_utils.cpp | 31 |
1 files changed, 15 insertions, 16 deletions
diff --git a/km_openssl/openssl_utils.cpp b/km_openssl/openssl_utils.cpp index b27c434..a0c0300 100644 --- a/km_openssl/openssl_utils.cpp +++ b/km_openssl/openssl_utils.cpp @@ -129,31 +129,30 @@ keymaster_error_t GetEcdsa256KeyFromCert(const keymaster_blob_t* km_cert, uint8_ return KM_ERROR_INVALID_ARGUMENT; } const uint8_t* temp = km_cert->data; - X509* cert = d2i_X509(NULL, &temp, km_cert->data_length); - if (cert == nullptr) return TranslateLastOpenSslError(); - EVP_PKEY* pubKey = X509_get_pubkey(cert); - if (pubKey == nullptr) return TranslateLastOpenSslError(); - EC_KEY* ecKey = EVP_PKEY_get0_EC_KEY(pubKey); - if (ecKey == nullptr) return TranslateLastOpenSslError(); + X509_Ptr cert(d2i_X509(NULL, &temp, km_cert->data_length)); + if (!cert.get()) return TranslateLastOpenSslError(); + EVP_PKEY_Ptr pubKey(X509_get_pubkey(cert.get())); + if (!pubKey.get()) return TranslateLastOpenSslError(); + EC_KEY* ecKey = EVP_PKEY_get0_EC_KEY(pubKey.get()); + if (!ecKey) return TranslateLastOpenSslError(); const EC_POINT* jacobian_coords = EC_KEY_get0_public_key(ecKey); - if (jacobian_coords == nullptr) return TranslateLastOpenSslError(); - BIGNUM x; - BIGNUM y; - BN_CTX* ctx = BN_CTX_new(); - if (ctx == nullptr) return TranslateLastOpenSslError(); - if (!EC_POINT_get_affine_coordinates_GFp(EC_KEY_get0_group(ecKey), jacobian_coords, &x, &y, - ctx)) { + if (!jacobian_coords) return TranslateLastOpenSslError(); + bssl::UniquePtr<BIGNUM> x(BN_new()); + bssl::UniquePtr<BIGNUM> y(BN_new()); + BN_CTX_Ptr ctx(BN_CTX_new()); + if (!ctx.get()) return TranslateLastOpenSslError(); + if (!EC_POINT_get_affine_coordinates_GFp(EC_KEY_get0_group(ecKey), jacobian_coords, x.get(), + y.get(), ctx.get())) { return TranslateLastOpenSslError(); } uint8_t* tmp_x = x_coord; - if (BN_bn2binpad(&x, tmp_x, kAffinePointLength) != kAffinePointLength) { + if (BN_bn2binpad(x.get(), tmp_x, kAffinePointLength) != kAffinePointLength) { return TranslateLastOpenSslError(); } uint8_t* tmp_y = y_coord; - if (BN_bn2binpad(&y, tmp_y, kAffinePointLength) != kAffinePointLength) { + if (BN_bn2binpad(y.get(), tmp_y, kAffinePointLength) != kAffinePointLength) { return TranslateLastOpenSslError(); } - BN_CTX_free(ctx); return KM_ERROR_OK; } |