Add buffer wrap checks and disable throwing of std::bad_alloc.
Android is built with exceptions disabled, but "operator new" and
"operator new[]" still throw std::bad_alloc on failure rather than
returning new. In general this is a good thing, because it will cause
an immediate crash of the process rather than assigning a null pointer
which is probably not checked. But most memory allocations in Keymaster
are checked, because it's written to run in an environment where new
does *not* throw. This CL updates the code to explicitly use the
non-throwing new.
A handful of throwing news remain, but only in places where a crash on
failure is appropriate.
In addition, this CL also inserts buffer wrap checks in key locations
and changes the development-machine Makefile to build in 32-bit mode, to
make memory problems more apparent.
Bug: 21888473
Change-Id: I7a906e9c757e25fcd8b1d58cede35cd57a336756
diff --git a/Makefile b/Makefile
index c41fa18..88131e8 100644
--- a/Makefile
+++ b/Makefile
@@ -34,20 +34,26 @@
-I $(BASE)/libnativehelper/include/nativehelper \
-I $(GTEST) -Iinclude -I$(BASE)/../boringssl/include
+ifdef FORCE_32_BIT
+ARCH_FLAGS = -m32
+endif
+
ifdef USE_CLANG
CC=/usr/bin/clang
CXX=/usr/bin/clang
-CLANG_TEST_DEFINE=-DKEYMASTER_CLANG_TEST_BUILD
-COMPILER_SPECIFIC_ARGS=-std=c++11 $(CLANG_TEST_DEFINE) -Wunused-private-field
+CXXFLAGS +=-std=c++11 -DKEYMASTER_CLANG_TEST_BUILD
+CFLAGS += -DKEYMASTER_CLANG_TEST_BUILD
else
-COMPILER_SPECIFIC_ARGS=-std=c++0x -fprofile-arcs
+CXXFLAGS +=-std=c++0x -fprofile-arcs
+CFLAGS += -fprofile-arcs
endif
-CPPFLAGS=$(INCLUDES) -g -O0 -MD -MP
-CXXFLAGS=-Wall -Werror -Wno-unused -Winit-self -Wpointer-arith -Wunused-parameter \
- -Werror=sign-compare -ftest-coverage -fno-permissive -Wunused-value \
- -Wno-deprecated-declarations -fno-exceptions -DKEYMASTER_NAME_TAGS \
- $(COMPILER_SPECIFIC_ARGS)
+LDFLAGS += $(ARCH_FLAGS)
+CPPFLAGS = $(INCLUDES) -g -O0 -MD -MP
+CXXFLAGS += -Wall -Werror -Wno-unused -Winit-self -Wpointer-arith -Wunused-parameter \
+ -Werror=sign-compare -ftest-coverage -fno-permissive \
+ -Wno-deprecated-declarations -fno-exceptions -DKEYMASTER_NAME_TAGS $(ARCH_FLAGS)
+CFLAGS += $(ARCH_FLAGS)
# Uncomment to enable debug logging.
# CXXFLAGS += -DDEBUG
@@ -165,6 +171,7 @@
hmac_test: hmac_test.o \
android_keymaster_test_utils.o \
+ android_keymaster_utils.o \
authorization_set.o \
hmac.o \
logger.o \
@@ -173,6 +180,7 @@
hkdf_test: hkdf_test.o \
android_keymaster_test_utils.o \
+ android_keymaster_utils.o \
authorization_set.o \
hkdf.o \
hmac.o \
@@ -192,6 +200,7 @@
android_keymaster_utils.o \
auth_encrypted_key_blob.o \
authorization_set.o \
+ integrity_assured_key_blob.o \
logger.o \
ocb.o \
ocb_utils.o \
@@ -258,7 +267,6 @@
$(GTEST_OBJS)
$(GTEST)/src/gtest-all.o: CXXFLAGS:=$(subst -Wmissing-declarations,,$(CXXFLAGS))
-ocb.o: CFLAGS=$(CLANG_TEST_DEFINE)
clean:
rm -f $(OBJS) $(DEPS) $(BINARIES) \
diff --git a/aes_key.cpp b/aes_key.cpp
index 1d59010..bc940df 100644
--- a/aes_key.cpp
+++ b/aes_key.cpp
@@ -18,6 +18,8 @@
#include <assert.h>
+#include <new>
+
#include <openssl/err.h>
#include <openssl/rand.h>
@@ -47,7 +49,7 @@
return KM_ERROR_OUTPUT_PARAMETER_NULL;
keymaster_error_t error = KM_ERROR_OK;
- key->reset(new AesKey(key_material, hw_enforced, sw_enforced, &error));
+ key->reset(new (std::nothrow) AesKey(key_material, hw_enforced, sw_enforced, &error));
if (!key->get())
error = KM_ERROR_MEMORY_ALLOCATION_FAILED;
return error;
diff --git a/aes_operation.cpp b/aes_operation.cpp
index 00c2dae..2325ad4 100644
--- a/aes_operation.cpp
+++ b/aes_operation.cpp
@@ -14,8 +14,12 @@
* limitations under the License.
*/
+#include "aes_operation.h"
+
#include <stdio.h>
+#include <new>
+
#include <UniquePtr.h>
#include <openssl/aes.h>
@@ -25,7 +29,6 @@
#include <keymaster/logger.h>
#include "aes_key.h"
-#include "aes_operation.h"
#include "openssl_err.h"
namespace keymaster {
@@ -96,12 +99,14 @@
Operation* op = NULL;
switch (purpose()) {
case KM_PURPOSE_ENCRYPT:
- op = new AesEvpEncryptOperation(block_mode, padding, caller_nonce, tag_length,
- symmetric_key->key_data(), symmetric_key->key_data_size());
+ op = new (std::nothrow)
+ AesEvpEncryptOperation(block_mode, padding, caller_nonce, tag_length,
+ symmetric_key->key_data(), symmetric_key->key_data_size());
break;
case KM_PURPOSE_DECRYPT:
- op = new AesEvpDecryptOperation(block_mode, padding, tag_length, symmetric_key->key_data(),
- symmetric_key->key_data_size());
+ op = new (std::nothrow)
+ AesEvpDecryptOperation(block_mode, padding, tag_length, symmetric_key->key_data(),
+ symmetric_key->key_data_size());
break;
default:
*error = KM_ERROR_UNSUPPORTED_PURPOSE;
@@ -147,7 +152,7 @@
AuthorizationSet* /* output_params */) {
if (block_mode_ == KM_MODE_GCM) {
aad_block_buf_length_ = 0;
- aad_block_buf_.reset(new uint8_t[AES_BLOCK_SIZE]);
+ aad_block_buf_.reset(new (std::nothrow) uint8_t[AES_BLOCK_SIZE]);
if (!aad_block_buf_.get())
return KM_ERROR_MEMORY_ALLOCATION_FAILED;
}
@@ -197,7 +202,8 @@
}
assert(output_written <= AES_BLOCK_SIZE);
- output->advance_write(output_written);
+ if (!output->advance_write(output_written))
+ return KM_ERROR_UNKNOWN_ERROR;
return KM_ERROR_OK;
}
@@ -299,7 +305,7 @@
if (block_mode_ == KM_MODE_GCM) {
aad_block_buf_length_ = 0;
- aad_block_buf_.reset(new uint8_t[AES_BLOCK_SIZE]);
+ aad_block_buf_.reset(new (std::nothrow) uint8_t[AES_BLOCK_SIZE]);
if (!aad_block_buf_.get())
return KM_ERROR_MEMORY_ALLOCATION_FAILED;
}
@@ -425,8 +431,7 @@
*error = TranslateLastOpenSslError();
return false;
}
- output->advance_write(output_written);
- return true;
+ return output->advance_write(output_written);
}
keymaster_error_t AesEvpEncryptOperation::Begin(const AuthorizationSet& input_params,
@@ -469,7 +474,8 @@
if (!EVP_CIPHER_CTX_ctrl(&ctx_, EVP_CTRL_GCM_GET_TAG, tag_length_, output->peek_write()))
return TranslateLastOpenSslError();
- output->advance_write(tag_length_);
+ if (!output->advance_write(tag_length_))
+ return KM_ERROR_UNKNOWN_ERROR;
}
return KM_ERROR_OK;
@@ -477,7 +483,7 @@
keymaster_error_t AesEvpEncryptOperation::GenerateIv() {
iv_length_ = (block_mode_ == KM_MODE_GCM) ? GCM_NONCE_SIZE : AES_BLOCK_SIZE;
- iv_.reset(new uint8_t[iv_length_]);
+ iv_.reset(new (std::nothrow) uint8_t[iv_length_]);
if (!iv_.get())
return KM_ERROR_MEMORY_ALLOCATION_FAILED;
if (RAND_bytes(iv_.get(), iv_length_) != 1)
@@ -495,7 +501,7 @@
if (tag_length_ > 0) {
tag_buf_length_ = 0;
- tag_buf_.reset(new uint8_t[tag_length_]);
+ tag_buf_.reset(new (std::nothrow) uint8_t[tag_length_]);
if (!tag_buf_.get())
return KM_ERROR_MEMORY_ALLOCATION_FAILED;
}
diff --git a/android_keymaster_messages_test.cpp b/android_keymaster_messages_test.cpp
index 5f53b87..913dd1c 100644
--- a/android_keymaster_messages_test.cpp
+++ b/android_keymaster_messages_test.cpp
@@ -575,6 +575,26 @@
msg.Deserialize(&p, end);
}
}
+
+ time_t now = time(NULL);
+ std::cout << "Seeding rand() with " << now << " for fuzz test." << std::endl;
+ srand(now);
+
+ // Fill large buffer with random bytes.
+ const int kBufSize = 10000;
+ UniquePtr<uint8_t[]> buf(new uint8_t[kBufSize]);
+ for (size_t i = 0; i < kBufSize; ++i)
+ buf[i] = static_cast<uint8_t>(rand());
+
+ for (uint32_t ver = 0; ver < MAX_MESSAGE_VERSION; ++ver) {
+ Message msg(ver);
+ const uint8_t* end = buf.get() + kBufSize;
+ for (size_t i = 0; i < kBufSize; ++i) {
+ const uint8_t* begin = buf.get() + i;
+ const uint8_t* p = begin;
+ msg.Deserialize(&p, end);
+ }
+ }
}
#define GARBAGE_TEST(Message) \
diff --git a/android_keymaster_test.cpp b/android_keymaster_test.cpp
index 542d430..0d4456d 100644
--- a/android_keymaster_test.cpp
+++ b/android_keymaster_test.cpp
@@ -587,7 +587,7 @@
EXPECT_EQ(31U, input_consumed);
string signature;
- ASSERT_EQ(KM_ERROR_UNKNOWN_ERROR, FinishOperation(&signature));
+ ASSERT_EQ(KM_ERROR_INVALID_INPUT_LENGTH, FinishOperation(&signature));
EXPECT_EQ(0U, signature.length());
if (GetParam()->algorithm_in_hardware(KM_ALGORITHM_RSA))
diff --git a/android_keymaster_utils.cpp b/android_keymaster_utils.cpp
index 7427b31..b0f3ad2 100644
--- a/android_keymaster_utils.cpp
+++ b/android_keymaster_utils.cpp
@@ -16,11 +16,13 @@
#include <keymaster/android_keymaster_utils.h>
+#include <new>
+
namespace keymaster {
uint8_t* dup_buffer(const void* buf, size_t size) {
- uint8_t* retval = new uint8_t[size];
- if (retval != NULL)
+ uint8_t* retval = new (std::nothrow) uint8_t[size];
+ if (retval)
memcpy(retval, buf, size);
return retval;
}
diff --git a/asymmetric_key.cpp b/asymmetric_key.cpp
index 9df320c..74751f7 100644
--- a/asymmetric_key.cpp
+++ b/asymmetric_key.cpp
@@ -16,6 +16,8 @@
#include "asymmetric_key.h"
+#include <new>
+
#include <openssl/x509.h>
#include "openssl_err.h"
@@ -38,7 +40,7 @@
if (*size <= 0)
return TranslateLastOpenSslError();
- material->reset(new uint8_t[*size]);
+ material->reset(new (std::nothrow) uint8_t[*size]);
uint8_t* tmp = material->get();
i2d_PrivateKey(pkey.get(), &tmp);
@@ -62,7 +64,7 @@
if (key_data_length <= 0)
return TranslateLastOpenSslError();
- material->reset(new uint8_t[key_data_length]);
+ material->reset(new (std::nothrow) uint8_t[key_data_length]);
if (material->get() == NULL)
return KM_ERROR_MEMORY_ALLOCATION_FAILED;
diff --git a/auth_encrypted_key_blob.cpp b/auth_encrypted_key_blob.cpp
index 64e0d63..e84c161 100644
--- a/auth_encrypted_key_blob.cpp
+++ b/auth_encrypted_key_blob.cpp
@@ -74,9 +74,8 @@
LOG_E("Failed to deserialize unversioned blob", 0);
return KM_ERROR_INVALID_KEY_BLOB;
}
- nonce->advance_write(OCB_NONCE_LENGTH);
- tag->advance_write(OCB_TAG_LENGTH);
-
+ if (!nonce->advance_write(OCB_NONCE_LENGTH) || !tag->advance_write(OCB_TAG_LENGTH))
+ return KM_ERROR_UNKNOWN_ERROR;
return KM_ERROR_OK;
}
@@ -85,10 +84,16 @@
AuthorizationSet* hw_enforced,
AuthorizationSet* sw_enforced, Buffer* nonce,
Buffer* tag) {
+ if (!key_blob.key_material || key_blob.key_material_size == 0)
+ return KM_ERROR_INVALID_KEY_BLOB;
+
const uint8_t* tmp = key_blob.key_material;
const uint8_t** buf_ptr = &tmp;
const uint8_t* end = tmp + key_blob.key_material_size;
+ if (end <= *buf_ptr)
+ return KM_ERROR_INVALID_KEY_BLOB;
+
uint8_t version = *(*buf_ptr)++;
if (version != CURRENT_BLOB_VERSION || //
!nonce->Deserialize(buf_ptr, end) || nonce->available_read() != OCB_NONCE_LENGTH ||
diff --git a/authorization_set.cpp b/authorization_set.cpp
index d860873..eef0710 100644
--- a/authorization_set.cpp
+++ b/authorization_set.cpp
@@ -14,13 +14,15 @@
* limitations under the License.
*/
+#include <keymaster/authorization_set.h>
+
+#include <assert.h>
#include <stdlib.h>
#include <string.h>
#include <stddef.h>
-#include <assert.h>
+#include <new>
-#include <keymaster/authorization_set.h>
#include <keymaster/android_keymaster_utils.h>
#include <keymaster/logger.h>
@@ -64,7 +66,7 @@
return false;
if (count >= elems_capacity_) {
- keymaster_key_param_t* new_elems = new keymaster_key_param_t[count];
+ keymaster_key_param_t* new_elems = new (std::nothrow) keymaster_key_param_t[count];
if (new_elems == NULL) {
set_invalid(ALLOCATION_FAILURE);
return false;
@@ -82,7 +84,7 @@
return false;
if (length > indirect_data_capacity_) {
- uint8_t* new_data = new uint8_t[length];
+ uint8_t* new_data = new (std::nothrow) uint8_t[length];
if (new_data == NULL) {
set_invalid(ALLOCATION_FAILURE);
return false;
@@ -389,7 +391,8 @@
// Note that the following validation of elements_count is weak, but it prevents allocation of
// elems_ arrays which are clearly too large to be reasonable.
if (static_cast<ptrdiff_t>(elements_size) > end - *buf_ptr ||
- elements_count * sizeof(uint32_t) > elements_size) {
+ elements_count * sizeof(uint32_t) > elements_size ||
+ *buf_ptr + (elements_count * sizeof(*elems_)) < *buf_ptr) {
LOG_E("Malformed data found in AuthorizationSet deserialization", 0);
set_invalid(MALFORMED_DATA);
return false;
diff --git a/ec_key_factory.cpp b/ec_key_factory.cpp
index e500a39..4ebe543 100644
--- a/ec_key_factory.cpp
+++ b/ec_key_factory.cpp
@@ -193,7 +193,7 @@
const AuthorizationSet& sw_enforced,
UniquePtr<AsymmetricKey>* key) const {
keymaster_error_t error;
- key->reset(new EcKey(hw_enforced, sw_enforced, &error));
+ key->reset(new (std::nothrow) EcKey(hw_enforced, sw_enforced, &error));
if (!key->get())
error = KM_ERROR_MEMORY_ALLOCATION_FAILED;
return error;
diff --git a/ec_keymaster0_key.cpp b/ec_keymaster0_key.cpp
index 5c5ea44..41af908 100644
--- a/ec_keymaster0_key.cpp
+++ b/ec_keymaster0_key.cpp
@@ -109,7 +109,8 @@
return KM_ERROR_UNKNOWN_ERROR;
keymaster_error_t error;
- key->reset(new EcKeymaster0Key(ec_key.release(), hw_enforced, sw_enforced, engine_, &error));
+ key->reset(new (std::nothrow)
+ EcKeymaster0Key(ec_key.release(), hw_enforced, sw_enforced, engine_, &error));
if (error != KM_ERROR_OK)
return error;
@@ -132,10 +133,9 @@
return KM_ERROR_UNKNOWN_ERROR;
*size = blob->key_material_size;
- material->reset(new uint8_t[*size]);
+ material->reset(dup_buffer(blob->key_material, *size));
if (!material->get())
return KM_ERROR_MEMORY_ALLOCATION_FAILED;
- memcpy(material->get(), blob->key_material, *size);
return KM_ERROR_OK;
}
diff --git a/ecdsa_operation.cpp b/ecdsa_operation.cpp
index dcbf73f..89bcfa1 100644
--- a/ecdsa_operation.cpp
+++ b/ecdsa_operation.cpp
@@ -156,7 +156,8 @@
if (EVP_DigestSignFinal(&digest_ctx_, output->peek_write(), &siglen) <= 0)
return TranslateLastOpenSslError();
}
- output->advance_write(siglen);
+ if (!output->advance_write(siglen))
+ return KM_ERROR_UNKNOWN_ERROR;
return KM_ERROR_OK;
}
diff --git a/ecdsa_operation.h b/ecdsa_operation.h
index 1c29c98..fdca143 100644
--- a/ecdsa_operation.h
+++ b/ecdsa_operation.h
@@ -88,7 +88,7 @@
private:
keymaster_purpose_t purpose() const override { return KM_PURPOSE_SIGN; }
Operation* InstantiateOperation(keymaster_digest_t digest, EVP_PKEY* key) {
- return new EcdsaSignOperation(purpose(), digest, key);
+ return new (std::nothrow) EcdsaSignOperation(purpose(), digest, key);
}
};
@@ -96,7 +96,7 @@
public:
keymaster_purpose_t purpose() const override { return KM_PURPOSE_VERIFY; }
Operation* InstantiateOperation(keymaster_digest_t digest, EVP_PKEY* key) {
- return new EcdsaVerifyOperation(KM_PURPOSE_VERIFY, digest, key);
+ return new (std::nothrow) EcdsaVerifyOperation(KM_PURPOSE_VERIFY, digest, key);
}
};
diff --git a/hkdf.cpp b/hkdf.cpp
index 331f8e5..19c663d 100644
--- a/hkdf.cpp
+++ b/hkdf.cpp
@@ -14,11 +14,16 @@
* limitations under the License.
*/
-#include "hmac.h"
#include "hkdf.h"
#include <assert.h>
+#include <new>
+
+#include <keymaster/android_keymaster_utils.h>
+
+#include "hmac.h"
+
namespace keymaster {
const size_t kSHA256HashLength = 32;
@@ -48,7 +53,7 @@
}
assert(result);
// avoid the unused variable warning if asserts are disabled.
- (void) result;
+ (void)result;
// |prk| is a pseudorandom key (of kSHA256HashLength octets).
uint8_t prk[kSHA256HashLength];
@@ -60,7 +65,7 @@
// https://tools.ietf.org/html/rfc5869#section-2.3
const size_t n = (key_bytes_to_generate + kSHA256HashLength - 1) / kSHA256HashLength;
assert(n < 256u);
- output_.reset(new uint8_t[n * kSHA256HashLength]);
+ output_.reset(new (std::nothrow) uint8_t[n * kSHA256HashLength]);
if (!output_.get()) {
*error = KM_ERROR_MEMORY_ALLOCATION_FAILED;
return;
@@ -88,12 +93,11 @@
if (key_bytes_to_generate) {
secret_key_len_ = key_bytes_to_generate;
- secret_key_.reset(new uint8_t[key_bytes_to_generate]);
+ secret_key_.reset(dup_buffer(output_.get(), key_bytes_to_generate));
if (!secret_key_.get()) {
*error = KM_ERROR_MEMORY_ALLOCATION_FAILED;
return;
}
- memcpy(secret_key_.get(), output_.get(), key_bytes_to_generate);
}
*error = KM_ERROR_OK;
}
diff --git a/hmac.cpp b/hmac.cpp
index 59c873c..02f739c 100644
--- a/hmac.cpp
+++ b/hmac.cpp
@@ -17,13 +17,13 @@
#include "hmac.h"
#include <assert.h>
+
#include <openssl/evp.h>
#include <openssl/hmac.h>
+#include <openssl/mem.h>
#include <openssl/sha.h>
-#if defined(OPENSSL_IS_BORINGSSL)
-#include <openssl/mem.h>
-#endif
+#include <keymaster/android_keymaster_utils.h>
namespace keymaster {
@@ -40,11 +40,10 @@
return false;
key_len_ = key_len;
- key_.reset(new uint8_t[key_len]);
+ key_.reset(dup_buffer(key, key_len));
if (!key_.get()) {
return false;
}
- memcpy(key_.get(), key, key_len);
return true;
}
diff --git a/hmac_key.cpp b/hmac_key.cpp
index ac6c482..2e814a6 100644
--- a/hmac_key.cpp
+++ b/hmac_key.cpp
@@ -16,6 +16,8 @@
#include "hmac_key.h"
+#include <new>
+
#include <openssl/err.h>
#include <openssl/rand.h>
@@ -45,7 +47,7 @@
return KM_ERROR_OUTPUT_PARAMETER_NULL;
keymaster_error_t error;
- key->reset(new HmacKey(key_material, hw_enforced, sw_enforced, &error));
+ key->reset(new (std::nothrow) HmacKey(key_material, hw_enforced, sw_enforced, &error));
if (!key->get())
error = KM_ERROR_MEMORY_ALLOCATION_FAILED;
return error;
diff --git a/hmac_operation.cpp b/hmac_operation.cpp
index 9ca7b96..75e8a07 100644
--- a/hmac_operation.cpp
+++ b/hmac_operation.cpp
@@ -16,6 +16,8 @@
#include "hmac_operation.h"
+#include <new>
+
#include <openssl/evp.h>
#include <openssl/hmac.h>
@@ -54,9 +56,9 @@
}
const SymmetricKey* symmetric_key = static_cast<const SymmetricKey*>(&key);
- UniquePtr<HmacOperation> op(new HmacOperation(purpose(), symmetric_key->key_data(),
- symmetric_key->key_data_size(), digest,
- tag_length / 8));
+ UniquePtr<HmacOperation> op(
+ new (std::nothrow) HmacOperation(purpose(), symmetric_key->key_data(),
+ symmetric_key->key_data_size(), digest, tag_length / 8));
if (!op.get())
*error = KM_ERROR_MEMORY_ALLOCATION_FAILED;
else
diff --git a/include/keymaster/android_keymaster_utils.h b/include/keymaster/android_keymaster_utils.h
index 16140a6..b957dd1 100644
--- a/include/keymaster/android_keymaster_utils.h
+++ b/include/keymaster/android_keymaster_utils.h
@@ -66,8 +66,8 @@
* responsibility.
*/
template <typename T> inline T* dup_array(const T* a, size_t n) {
- T* dup = new T[n];
- if (dup != NULL)
+ T* dup = new (std::nothrow) T[n];
+ if (dup)
for (size_t i = 0; i < n; ++i)
dup[i] = a[i];
return dup;
@@ -218,23 +218,31 @@
}
KeymasterKeyBlob(const uint8_t* data, size_t size) {
+ key_material_size = 0;
key_material = dup_buffer(data, size);
- key_material_size = size;
+ if (key_material)
+ key_material_size = size;
}
explicit KeymasterKeyBlob(size_t size) {
- key_material = new uint8_t[size];
- key_material_size = size;
+ key_material_size = 0;
+ key_material = new (std::nothrow) uint8_t[size];
+ if (key_material)
+ key_material_size = size;
}
explicit KeymasterKeyBlob(const keymaster_key_blob_t& blob) {
+ key_material_size = 0;
key_material = dup_buffer(blob.key_material, blob.key_material_size);
- key_material_size = blob.key_material_size;
+ if (key_material)
+ key_material_size = blob.key_material_size;
}
KeymasterKeyBlob(const KeymasterKeyBlob& blob) {
+ key_material_size = 0;
key_material = dup_buffer(blob.key_material, blob.key_material_size);
- key_material_size = blob.key_material_size;
+ if (key_material)
+ key_material_size = blob.key_material_size;
}
void operator=(const KeymasterKeyBlob& blob) {
@@ -257,8 +265,9 @@
const uint8_t* Reset(size_t new_size) {
Clear();
- key_material = new uint8_t[new_size];
- key_material_size = new_size;
+ key_material = new (std::nothrow) uint8_t[new_size];
+ if (key_material)
+ key_material_size = new_size;
return key_material;
}
diff --git a/include/keymaster/serializable.h b/include/keymaster/serializable.h
index e996fd8..a7cce83 100644
--- a/include/keymaster/serializable.h
+++ b/include/keymaster/serializable.h
@@ -22,6 +22,7 @@
#include <string.h>
#include <cstddef>
+#include <new>
#include <UniquePtr.h>
@@ -109,6 +110,9 @@
template <typename T>
inline uint8_t* append_uint32_array_to_buf(uint8_t* buf, const uint8_t* end, const T* data,
size_t count) {
+ // Check for overflow
+ if (count >= (UINT32_MAX / sizeof(uint32_t)) || buf + count * sizeof(uint32_t) < buf)
+ return buf;
buf = append_uint32_to_buf(buf, end, count);
for (size_t i = 0; i < count; ++i)
buf = append_uint32_to_buf(buf, end, static_cast<uint32_t>(data[i]));
@@ -165,9 +169,16 @@
template <typename T>
inline bool copy_uint32_array_from_buf(const uint8_t** buf_ptr, const uint8_t* end,
UniquePtr<T[]>* data, size_t* count) {
- if (!copy_uint32_from_buf(buf_ptr, end, count) || *buf_ptr + *count * sizeof(uint32_t) > end)
+ if (!copy_uint32_from_buf(buf_ptr, end, count))
return false;
- data->reset(new T[*count]);
+
+ const uint8_t* array_end = *buf_ptr + *count * sizeof(uint32_t);
+ if (array_end < *buf_ptr || array_end > end)
+ return false;
+
+ data->reset(new (std::nothrow) T[*count]);
+ if (!data->get())
+ return false;
for (size_t i = 0; i < *count; ++i)
if (!copy_uint32_from_buf(buf_ptr, end, &(*data)[i]))
return false;
@@ -206,9 +217,21 @@
bool write(const uint8_t* src, size_t write_length);
bool read(uint8_t* dest, size_t read_length);
const uint8_t* peek_read() const { return buffer_.get() + read_position_; }
- void advance_read(int distance) { read_position_ += distance; }
+ bool advance_read(int distance) {
+ if (static_cast<size_t>(read_position_ + distance) <= write_position_) {
+ read_position_ += distance;
+ return true;
+ }
+ return false;
+ }
uint8_t* peek_write() { return buffer_.get() + write_position_; }
- void advance_write(int distance) { write_position_ += distance; }
+ bool advance_write(int distance) {
+ if (static_cast<size_t>(write_position_ + distance) <= buffer_size_) {
+ write_position_ += distance;
+ return true;
+ }
+ return false;
+ }
size_t SerializedSize() const;
uint8_t* Serialize(uint8_t* buf, const uint8_t* end) const;
@@ -221,8 +244,8 @@
UniquePtr<uint8_t[]> buffer_;
size_t buffer_size_;
- int read_position_;
- int write_position_;
+ size_t read_position_;
+ size_t write_position_;
};
} // namespace keymaster
diff --git a/integrity_assured_key_blob.cpp b/integrity_assured_key_blob.cpp
index 9ae349d..83ca432 100644
--- a/integrity_assured_key_blob.cpp
+++ b/integrity_assured_key_blob.cpp
@@ -18,6 +18,8 @@
#include <assert.h>
+#include <new>
+
#include <openssl/hmac.h>
#include <openssl/mem.h>
@@ -50,7 +52,9 @@
static keymaster_error_t ComputeHmac(const uint8_t* serialized_data, size_t serialized_data_size,
const AuthorizationSet& hidden, uint8_t hmac[HMAC_SIZE]) {
size_t hidden_bytes_size = hidden.SerializedSize();
- UniquePtr<uint8_t[]> hidden_bytes(new uint8_t[hidden_bytes_size]);
+ UniquePtr<uint8_t[]> hidden_bytes(new (std::nothrow) uint8_t[hidden_bytes_size]);
+ if (!hidden_bytes.get())
+ return KM_ERROR_MEMORY_ALLOCATION_FAILED;
hidden.Serialize(hidden_bytes.get(), hidden_bytes.get() + hidden_bytes_size);
HMAC_CTX ctx;
@@ -104,6 +108,9 @@
const uint8_t* p = key_blob.begin();
const uint8_t* end = key_blob.end();
+ if (p > end || p + HMAC_SIZE > end)
+ return KM_ERROR_INVALID_KEY_BLOB;
+
uint8_t computed_hmac[HMAC_SIZE];
keymaster_error_t error = ComputeHmac(key_blob.begin(), key_blob.key_material_size - HMAC_SIZE,
hidden, computed_hmac);
diff --git a/key_blob_test.cpp b/key_blob_test.cpp
index f54197d..20349c0 100644
--- a/key_blob_test.cpp
+++ b/key_blob_test.cpp
@@ -27,6 +27,7 @@
#include "android_keymaster_test_utils.h"
#include "auth_encrypted_key_blob.h"
+#include "integrity_assured_key_blob.h"
#include "ocb_utils.h"
namespace keymaster {
@@ -287,5 +288,71 @@
nonce_, tag_, &decrypted_plaintext_));
}
+// This test is especially useful when compiled for 32-bit mode and run under valgrind.
+TEST_F(KeyBlobTest, FuzzTest) {
+ time_t now = time(NULL);
+ std::cout << "Seeding rand() with " << now << " for fuzz test." << std::endl;
+ srand(now);
+
+ // Fill large buffer with random bytes.
+ const int kBufSize = 10000;
+ UniquePtr<uint8_t[]> buf(new uint8_t[kBufSize]);
+ for (size_t i = 0; i < kBufSize; ++i)
+ buf[i] = static_cast<uint8_t>(rand());
+
+ // Try to deserialize every offset with multiple methods.
+ size_t deserialize_auth_encrypted_success = 0;
+ for (size_t i = 0; i < kBufSize; ++i) {
+ keymaster_key_blob_t blob = {buf.get() + i, kBufSize - i};
+ KeymasterKeyBlob key_blob(blob);
+
+ // Integrity-assured blob.
+ ASSERT_EQ(KM_ERROR_INVALID_KEY_BLOB,
+ DeserializeIntegrityAssuredBlob(key_blob, hidden_, &key_material_, &hw_enforced_,
+ &sw_enforced_));
+
+ // Auth-encrypted OCB blob.
+ keymaster_error_t error = DeserializeAuthEncryptedBlob(
+ key_blob, &ciphertext_, &hw_enforced_, &sw_enforced_, &nonce_, &tag_);
+ if (error == KM_ERROR_OK) {
+ // It's possible to deserialize successfully. Decryption should always fail.
+ ++deserialize_auth_encrypted_success;
+ error = OcbDecryptKey(hw_enforced_, sw_enforced_, hidden_, master_key_, ciphertext_,
+ nonce_, tag_, &decrypted_plaintext_);
+ }
+ ASSERT_EQ(KM_ERROR_INVALID_KEY_BLOB, error)
+ << "Somehow sucessfully parsed a blob with seed " << now << " at offset " << i;
+ }
+}
+
+TEST_F(KeyBlobTest, UnderflowTest) {
+ uint8_t buf[0];
+ keymaster_key_blob_t blob = {buf, 0};
+ KeymasterKeyBlob key_blob1(blob);
+ EXPECT_NE(nullptr, key_blob1.key_material);
+ EXPECT_EQ(0U, key_blob1.key_material_size);
+
+ EXPECT_EQ(KM_ERROR_INVALID_KEY_BLOB,
+ DeserializeIntegrityAssuredBlob(key_blob1, hidden_, &key_material_, &hw_enforced_,
+ &sw_enforced_));
+
+ EXPECT_EQ(KM_ERROR_INVALID_KEY_BLOB,
+ DeserializeAuthEncryptedBlob(key_blob1, &ciphertext_, &hw_enforced_, &sw_enforced_,
+ &nonce_, &tag_));
+
+ blob.key_material_size = UINT32_MAX;
+ KeymasterKeyBlob key_blob2(blob);
+ EXPECT_EQ(nullptr, key_blob2.key_material);
+ EXPECT_EQ(0U, key_blob2.key_material_size);
+
+ ASSERT_EQ(KM_ERROR_INVALID_KEY_BLOB,
+ DeserializeIntegrityAssuredBlob(key_blob2, hidden_, &key_material_, &hw_enforced_,
+ &sw_enforced_));
+
+ EXPECT_EQ(KM_ERROR_INVALID_KEY_BLOB,
+ DeserializeAuthEncryptedBlob(key_blob2, &ciphertext_, &hw_enforced_, &sw_enforced_,
+ &nonce_, &tag_));
+}
+
} // namespace test
} // namespace keymaster
diff --git a/keymaster0_engine.cpp b/keymaster0_engine.cpp
index 900bb7e..4ee5f9c 100644
--- a/keymaster0_engine.cpp
+++ b/keymaster0_engine.cpp
@@ -162,12 +162,13 @@
}
static keymaster_key_blob_t* duplicate_blob(const uint8_t* key_data, size_t key_data_size) {
- unique_ptr<uint8_t[]> key_material_copy(new uint8_t[key_data_size]);
+ unique_ptr<uint8_t[]> key_material_copy(dup_buffer(key_data, key_data_size));
if (!key_material_copy)
return nullptr;
- memcpy(key_material_copy.get(), key_data, key_data_size);
- unique_ptr<keymaster_key_blob_t> blob_copy(new keymaster_key_blob_t);
+ unique_ptr<keymaster_key_blob_t> blob_copy(new (std::nothrow) keymaster_key_blob_t);
+ if (!blob_copy.get())
+ return nullptr;
blob_copy->key_material_size = key_data_size;
blob_copy->key_material = key_material_copy.release();
return blob_copy.release();
diff --git a/ocb_utils.cpp b/ocb_utils.cpp
index 171d07e..7038da0 100644
--- a/ocb_utils.cpp
+++ b/ocb_utils.cpp
@@ -18,6 +18,8 @@
#include <assert.h>
+#include <new>
+
#include <openssl/aes.h>
#include <openssl/sha.h>
@@ -50,7 +52,7 @@
size_t* derivation_data_length) {
*derivation_data_length =
hidden.SerializedSize() + hw_enforced.SerializedSize() + sw_enforced.SerializedSize();
- derivation_data->reset(new uint8_t[*derivation_data_length]);
+ derivation_data->reset(new (std::nothrow) uint8_t[*derivation_data_length]);
if (!derivation_data->get())
return KM_ERROR_MEMORY_ALLOCATION_FAILED;
@@ -76,9 +78,13 @@
return error;
SHA256_CTX sha256_ctx;
- UniquePtr<uint8_t[]> hash_buf(new uint8_t[SHA256_DIGEST_LENGTH]);
+ UniquePtr<uint8_t[]> hash_buf(new (std::nothrow) uint8_t[SHA256_DIGEST_LENGTH]);
+ if (!hash_buf.get())
+ return KM_ERROR_MEMORY_ALLOCATION_FAILED;
Eraser hash_eraser(hash_buf.get(), SHA256_DIGEST_LENGTH);
- UniquePtr<uint8_t[]> derived_key(new uint8_t[AES_BLOCK_SIZE]);
+ UniquePtr<uint8_t[]> derived_key(new (std::nothrow) uint8_t[AES_BLOCK_SIZE]);
+ if (!derived_key.get())
+ return KM_ERROR_MEMORY_ALLOCATION_FAILED;
Eraser derived_key_eraser(derived_key.get(), AES_BLOCK_SIZE);
if (!ctx->get() || !hash_buf.get() || !derived_key.get())
@@ -120,13 +126,15 @@
return KM_ERROR_INVALID_ARGUMENT;
AeCtx ctx;
+ if (!ctx.get())
+ return KM_ERROR_MEMORY_ALLOCATION_FAILED;
+
keymaster_error_t error =
InitializeKeyWrappingContext(hw_enforced, sw_enforced, hidden, master_key, &ctx);
if (error != KM_ERROR_OK)
return error;
- ciphertext->Reset(plaintext.key_material_size);
- if (!ciphertext->key_material)
+ if (!ciphertext->Reset(plaintext.key_material_size))
return KM_ERROR_MEMORY_ALLOCATION_FAILED;
int ae_err = ae_encrypt(ctx.get(), nonce.peek_read(), plaintext.key_material,
@@ -137,7 +145,8 @@
LOG_E("Error %d while encrypting key", ae_err);
return KM_ERROR_UNKNOWN_ERROR;
}
- tag->advance_write(OCB_TAG_LENGTH);
+ if (!tag->advance_write(OCB_TAG_LENGTH))
+ return KM_ERROR_UNKNOWN_ERROR;
assert(ae_err == static_cast<int>(plaintext.key_material_size));
return KM_ERROR_OK;
}
@@ -153,13 +162,15 @@
return KM_ERROR_INVALID_ARGUMENT;
AeCtx ctx;
+ if (!ctx.get())
+ return KM_ERROR_MEMORY_ALLOCATION_FAILED;
+
keymaster_error_t error =
InitializeKeyWrappingContext(hw_enforced, sw_enforced, hidden, master_key, &ctx);
if (error != KM_ERROR_OK)
return error;
- plaintext->Reset(ciphertext.key_material_size);
- if (!plaintext->key_material)
+ if (!plaintext->Reset(ciphertext.key_material_size))
return KM_ERROR_MEMORY_ALLOCATION_FAILED;
int ae_err = ae_decrypt(ctx.get(), nonce.peek_read(), ciphertext.key_material,
diff --git a/openssl_utils.cpp b/openssl_utils.cpp
index e7048e4..3d29c86 100644
--- a/openssl_utils.cpp
+++ b/openssl_utils.cpp
@@ -22,12 +22,6 @@
namespace keymaster {
-void convert_bn_to_blob(BIGNUM* bn, keymaster_blob_t* blob) {
- blob->data_length = BN_num_bytes(bn);
- blob->data = new uint8_t[blob->data_length];
- BN_bn2bin(bn, const_cast<uint8_t*>(blob->data));
-}
-
static int convert_to_evp(keymaster_algorithm_t algorithm) {
switch (algorithm) {
case KM_ALGORITHM_RSA:
@@ -79,8 +73,7 @@
if (key_data_size <= 0)
return TranslateLastOpenSslError();
- key_blob->Reset(key_data_size);
- if (!key_blob->key_material)
+ if (!key_blob->Reset(key_data_size))
return KM_ERROR_MEMORY_ALLOCATION_FAILED;
uint8_t* tmp = key_blob->writable_data();
diff --git a/openssl_utils.h b/openssl_utils.h
index 33e4504..304c241 100644
--- a/openssl_utils.h
+++ b/openssl_utils.h
@@ -65,8 +65,6 @@
T* val __attribute__((unused)) = p.release();
}
-void convert_bn_to_blob(BIGNUM* bn, keymaster_blob_t* blob);
-
keymaster_error_t convert_pkcs8_blob_to_evp(const uint8_t* key_data, size_t key_length,
keymaster_algorithm_t expected_algorithm,
UniquePtr<EVP_PKEY, EVP_PKEY_Delete>* pkey);
diff --git a/operation_table.cpp b/operation_table.cpp
index e4b9e33..d30885e 100644
--- a/operation_table.cpp
+++ b/operation_table.cpp
@@ -16,6 +16,8 @@
#include "operation_table.h"
+#include <new>
+
#include <openssl/rand.h>
#include "openssl_err.h"
@@ -32,7 +34,7 @@
keymaster_error_t OperationTable::Add(Operation* operation,
keymaster_operation_handle_t* op_handle) {
if (!table_.get()) {
- table_.reset(new Entry[table_size_]);
+ table_.reset(new (std::nothrow) Entry[table_size_]);
if (!table_.get())
return KM_ERROR_MEMORY_ALLOCATION_FAILED;
}
diff --git a/rsa_key_factory.cpp b/rsa_key_factory.cpp
index a0fa782..dfe2ddd 100644
--- a/rsa_key_factory.cpp
+++ b/rsa_key_factory.cpp
@@ -16,6 +16,8 @@
#include <keymaster/rsa_key_factory.h>
+#include <new>
+
#include <keymaster/keymaster_context.h>
#include "openssl_err.h"
@@ -163,7 +165,7 @@
const AuthorizationSet& sw_enforced,
UniquePtr<AsymmetricKey>* key) const {
keymaster_error_t error;
- key->reset(new RsaKey(hw_enforced, sw_enforced, &error));
+ key->reset(new (std::nothrow) RsaKey(hw_enforced, sw_enforced, &error));
if (!key->get())
error = KM_ERROR_MEMORY_ALLOCATION_FAILED;
return error;
diff --git a/rsa_keymaster0_key.cpp b/rsa_keymaster0_key.cpp
index a6e2da3..b7ca064 100644
--- a/rsa_keymaster0_key.cpp
+++ b/rsa_keymaster0_key.cpp
@@ -34,8 +34,7 @@
RsaKeymaster0KeyFactory::RsaKeymaster0KeyFactory(const SoftKeymasterContext* context,
const Keymaster0Engine* engine)
- : RsaKeyFactory(context), engine_(engine) {
-}
+ : RsaKeyFactory(context), engine_(engine) {}
keymaster_error_t RsaKeymaster0KeyFactory::GenerateKey(const AuthorizationSet& key_description,
KeymasterKeyBlob* key_blob,
@@ -122,7 +121,11 @@
return KM_ERROR_UNKNOWN_ERROR;
keymaster_error_t error;
- key->reset(new RsaKeymaster0Key(rsa.release(), hw_enforced, sw_enforced, engine_, &error));
+ key->reset(new (std::nothrow)
+ RsaKeymaster0Key(rsa.release(), hw_enforced, sw_enforced, engine_, &error));
+ if (!key->get())
+ error = KM_ERROR_MEMORY_ALLOCATION_FAILED;
+
if (error != KM_ERROR_OK)
return error;
@@ -132,8 +135,7 @@
RsaKeymaster0Key::RsaKeymaster0Key(RSA* rsa_key, const AuthorizationSet& hw_enforced,
const AuthorizationSet& sw_enforced,
const Keymaster0Engine* engine, keymaster_error_t* error)
- : RsaKey(rsa_key, hw_enforced, sw_enforced, error), engine_(engine) {
-}
+ : RsaKey(rsa_key, hw_enforced, sw_enforced, error), engine_(engine) {}
keymaster_error_t RsaKeymaster0Key::key_material(UniquePtr<uint8_t[]>* material,
size_t* size) const {
@@ -145,10 +147,9 @@
return KM_ERROR_UNKNOWN_ERROR;
*size = blob->key_material_size;
- material->reset(new uint8_t[*size]);
+ material->reset(dup_buffer(blob->key_material, *size));
if (!material->get())
return KM_ERROR_MEMORY_ALLOCATION_FAILED;
- memcpy(material->get(), blob->key_material, *size);
return KM_ERROR_OK;
}
diff --git a/rsa_operation.cpp b/rsa_operation.cpp
index edbaa9e..c99fbfc 100644
--- a/rsa_operation.cpp
+++ b/rsa_operation.cpp
@@ -18,6 +18,8 @@
#include <limits.h>
+#include <new>
+
#include <openssl/err.h>
#include <keymaster/logger.h>
@@ -290,8 +292,9 @@
}
if (bytes_encrypted <= 0)
+ return TranslateLastOpenSslError();
+ if (!output->advance_write(bytes_encrypted))
return KM_ERROR_UNKNOWN_ERROR;
- output->advance_write(bytes_encrypted);
return KM_ERROR_OK;
}
@@ -305,7 +308,8 @@
if (EVP_DigestSignFinal(&digest_ctx_, output->peek_write(), &siglen) <= 0)
return TranslateLastOpenSslError();
- output->advance_write(siglen);
+ if (!output->advance_write(siglen))
+ return KM_ERROR_UNKNOWN_ERROR;
return KM_ERROR_OK;
}
@@ -371,7 +375,9 @@
return KM_ERROR_UNSUPPORTED_PADDING_MODE;
}
- UniquePtr<uint8_t[]> decrypted_data(new uint8_t[key_len]);
+ UniquePtr<uint8_t[]> decrypted_data(new (std::nothrow) uint8_t[key_len]);
+ if (!decrypted_data.get())
+ return KM_ERROR_MEMORY_ALLOCATION_FAILED;
int bytes_decrypted = RSA_public_decrypt(signature.available_read(), signature.peek_read(),
decrypted_data.get(), rsa.get(), openssl_padding);
if (bytes_decrypted < 0)
@@ -435,7 +441,8 @@
if (EVP_PKEY_encrypt(ctx.get(), output->peek_write(), &outlen, data_.peek_read(),
data_.available_read()) <= 0)
return TranslateLastOpenSslError();
- output->advance_write(outlen);
+ if (!output->advance_write(outlen))
+ return KM_ERROR_UNKNOWN_ERROR;
return KM_ERROR_OK;
}
@@ -469,7 +476,8 @@
if (EVP_PKEY_decrypt(ctx.get(), output->peek_write(), &outlen, data_.peek_read(),
data_.available_read()) <= 0)
return TranslateLastOpenSslError();
- output->advance_write(outlen);
+ if (!output->advance_write(outlen))
+ return KM_ERROR_UNKNOWN_ERROR;
return KM_ERROR_OK;
}
diff --git a/rsa_operation.h b/rsa_operation.h
index 4005dbd..a1de04c 100644
--- a/rsa_operation.h
+++ b/rsa_operation.h
@@ -209,7 +209,7 @@
keymaster_purpose_t purpose() const override { return KM_PURPOSE_SIGN; }
Operation* InstantiateOperation(keymaster_digest_t digest, keymaster_padding_t padding,
EVP_PKEY* key) override {
- return new RsaSignOperation(digest, padding, key);
+ return new (std::nothrow) RsaSignOperation(digest, padding, key);
}
};
@@ -220,7 +220,7 @@
keymaster_purpose_t purpose() const override { return KM_PURPOSE_VERIFY; }
Operation* InstantiateOperation(keymaster_digest_t digest, keymaster_padding_t padding,
EVP_PKEY* key) override {
- return new RsaVerifyOperation(digest, padding, key);
+ return new (std::nothrow) RsaVerifyOperation(digest, padding, key);
}
};
@@ -230,7 +230,7 @@
class RsaEncryptionOperationFactory : public RsaCryptingOperationFactory {
keymaster_purpose_t purpose() const override { return KM_PURPOSE_ENCRYPT; }
Operation* InstantiateOperation(keymaster_padding_t padding, EVP_PKEY* key) override {
- return new RsaEncryptOperation(padding, key);
+ return new (std::nothrow) RsaEncryptOperation(padding, key);
}
};
@@ -240,7 +240,7 @@
class RsaDecryptionOperationFactory : public RsaCryptingOperationFactory {
keymaster_purpose_t purpose() const override { return KM_PURPOSE_DECRYPT; }
Operation* InstantiateOperation(keymaster_padding_t padding, EVP_PKEY* key) override {
- return new RsaDecryptOperation(padding, key);
+ return new (std::nothrow) RsaDecryptOperation(padding, key);
}
};
diff --git a/serializable.cpp b/serializable.cpp
index e32b5a0..5db64f8 100644
--- a/serializable.cpp
+++ b/serializable.cpp
@@ -15,17 +15,30 @@
*/
#include <keymaster/serializable.h>
+
+#include <assert.h>
+
+#include <new>
+
#include <keymaster/android_keymaster_utils.h>
namespace keymaster {
uint8_t* append_to_buf(uint8_t* buf, const uint8_t* end, const void* data, size_t data_len) {
- if (buf + data_len <= end)
+ if (buf + data_len < buf) // Pointer wrap check
+ return buf;
+
+ if (buf + data_len <= end) {
memcpy(buf, data, data_len);
- return buf + data_len;
+ return buf + data_len;
+ }
+ return buf;
}
bool copy_from_buf(const uint8_t** buf_ptr, const uint8_t* end, void* dest, size_t size) {
+ if (*buf_ptr + size < *buf_ptr) // Pointer wrap check
+ return false;
+
if (end < *buf_ptr + size)
return false;
memcpy(dest, *buf_ptr, size);
@@ -35,15 +48,21 @@
bool copy_size_and_data_from_buf(const uint8_t** buf_ptr, const uint8_t* end, size_t* size,
UniquePtr<uint8_t[]>* dest) {
- if (!copy_uint32_from_buf(buf_ptr, end, size) || *buf_ptr + *size > end) {
+ if (!copy_uint32_from_buf(buf_ptr, end, size))
return false;
- }
+
+ if (*buf_ptr + *size < *buf_ptr) // Pointer wrap check
+ return false;
+
+ if (*buf_ptr + *size > end)
+ return false;
+
if (*size == 0) {
dest->reset();
return true;
}
- dest->reset(new uint8_t[*size]);
- if (dest->get() == NULL)
+ dest->reset(new (std::nothrow) uint8_t[*size]);
+ if (!dest->get())
return false;
return copy_from_buf(buf_ptr, end, dest->get(), *size);
}
@@ -51,7 +70,7 @@
bool Buffer::reserve(size_t size) {
if (available_write() < size) {
size_t new_size = buffer_size_ + size - available_write();
- uint8_t* new_buffer = new uint8_t[new_size];
+ uint8_t* new_buffer = new (std::nothrow) uint8_t[new_size];
if (!new_buffer)
return false;
memcpy(new_buffer, buffer_.get() + read_position_, available_read());
@@ -66,8 +85,8 @@
bool Buffer::Reinitialize(size_t size) {
Clear();
- buffer_.reset(new uint8_t[size]);
- if (buffer_.get() == NULL)
+ buffer_.reset(new (std::nothrow) uint8_t[size]);
+ if (!buffer_.get())
return false;
buffer_size_ = size;
read_position_ = 0;
@@ -77,8 +96,10 @@
bool Buffer::Reinitialize(const void* data, size_t data_len) {
Clear();
- buffer_.reset(new uint8_t[data_len]);
- if (buffer_.get() == NULL)
+ if (static_cast<const uint8_t*>(data) + data_len < data) // Pointer wrap check
+ return false;
+ buffer_.reset(new (std::nothrow) uint8_t[data_len]);
+ if (!buffer_.get())
return false;
buffer_size_ = data_len;
memcpy(buffer_.get(), data, data_len);
@@ -88,10 +109,13 @@
}
size_t Buffer::available_write() const {
+ assert(buffer_size_ >= write_position_);
return buffer_size_ - write_position_;
}
size_t Buffer::available_read() const {
+ assert(buffer_size_ >= write_position_);
+ assert(write_position_ >= read_position_);
return write_position_ - read_position_;
}
diff --git a/symmetric_key.cpp b/symmetric_key.cpp
index 016b40e..3d8424c 100644
--- a/symmetric_key.cpp
+++ b/symmetric_key.cpp
@@ -123,7 +123,7 @@
keymaster_error_t SymmetricKey::key_material(UniquePtr<uint8_t[]>* key_material,
size_t* size) const {
*size = key_data_size_;
- key_material->reset(new uint8_t[*size]);
+ key_material->reset(new (std::nothrow) uint8_t[*size]);
if (!key_material->get())
return KM_ERROR_MEMORY_ALLOCATION_FAILED;
memcpy(key_material->get(), key_data_.get(), *size);
