Skip to content

Commit

Permalink
MONGOCRYPT-18 coverity fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
kevinAlbs committed Nov 15, 2019
1 parent b68aac3 commit f602d75
Show file tree
Hide file tree
Showing 33 changed files with 258 additions and 56 deletions.
4 changes: 4 additions & 0 deletions kms-message/src/hexlify.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ char *
hexlify (const uint8_t *buf, size_t len)
{
char *hex_chars = malloc (len * 2 + 1);
KMS_ASSERT (hex_chars);

char *p = hex_chars;
size_t i;

Expand All @@ -44,6 +46,8 @@ unhexlify (const char *hex_chars, size_t *len)

*len = strlen (hex_chars) / 2;
buf = malloc (*len);
KMS_ASSERT (buf);

pos = buf;

while (*hex_chars) {
Expand Down
2 changes: 1 addition & 1 deletion kms-message/src/kms_decrypt_request.c
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ kms_decrypt_request_new (const uint8_t *ciphertext_blob,
if (!(b64 = malloc (b64_len))) {
KMS_ERROR (request,
"Could not allocate %d bytes for base64-encoding payload",
b64_len);
(int) b64_len);
goto done;
}

Expand Down
2 changes: 1 addition & 1 deletion kms-message/src/kms_encrypt_request.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ kms_encrypt_request_new (const uint8_t *plaintext,
if (!(b64 = malloc (b64_len))) {
KMS_ERROR (request,
"Could not allocate %d bytes for base64-encoding payload",
b64_len);
(int) b64_len);
goto done;
}

Expand Down
8 changes: 8 additions & 0 deletions kms-message/src/kms_kv_list.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include "kms_kv_list.h"
#include "kms_message/kms_message.h"
#include "kms_message_private.h"
#include "kms_request_str.h"
#include "kms_port.h"
#include "sort.h"
Expand All @@ -39,9 +40,12 @@ kms_kv_list_t *
kms_kv_list_new (void)
{
kms_kv_list_t *lst = malloc (sizeof (kms_kv_list_t));
KMS_ASSERT (lst);

lst->size = 16;
lst->kvs = malloc (lst->size * sizeof (kms_kv_t));
KMS_ASSERT (lst->kvs);

lst->len = 0;

return lst;
Expand Down Expand Up @@ -119,8 +123,12 @@ kms_kv_list_dup (const kms_kv_list_t *lst)
}

dup = malloc (sizeof (kms_kv_list_t));
KMS_ASSERT (dup);

dup->size = dup->len = lst->len;
dup->kvs = malloc (lst->len * sizeof (kms_kv_t));
KMS_ASSERT (dup->kvs);


for (i = 0; i < lst->len; i++) {
kv_init (&dup->kvs[i], lst->kvs[i].key, lst->kvs[i].value);
Expand Down
25 changes: 17 additions & 8 deletions kms-message/src/kms_request.c
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,14 @@ kms_request_new (const char *method,
request->header_fields = kms_kv_list_new ();
request->auto_content_length = true;

kms_request_set_date (request, NULL);
if (!kms_request_set_date (request, NULL)) {
return request;
}

if (opt && opt->connection_close) {
kms_request_add_header_field (request, "Connection", "close");
if (!kms_request_add_header_field (request, "Connection", "close")) {
return request;
}
}

if (opt && opt->crypto.sha256) {
Expand Down Expand Up @@ -164,7 +168,9 @@ kms_request_set_date (kms_request_t *request, const struct tm *tm)
kms_request_str_set_chars (request->date, buf, sizeof "YYYYmmDD" - 1);
kms_request_str_set_chars (request->datetime, buf, sizeof AMZ_DT_FORMAT - 1);
kms_kv_list_del (request->header_fields, "X-Amz-Date");
kms_request_add_header_field (request, "X-Amz-Date", buf);
if (!kms_request_add_header_field (request, "X-Amz-Date", buf)) {
return false;
}

return true;
}
Expand Down Expand Up @@ -447,19 +453,22 @@ kms_request_get_canonical (kms_request_t *request)
kms_request_str_append_newline (canonical);
normalized = kms_request_str_path_normalized (request->path);
kms_request_str_append_escaped (canonical, normalized, false);
kms_request_str_destroy (normalized);
kms_request_str_append_newline (canonical);
append_canonical_query (request, canonical);
kms_request_str_append_newline (canonical);
lst = canonical_headers (request);
append_canonical_headers (lst, canonical);
kms_request_str_append_newline (canonical);
append_signed_headers (lst, canonical);
kms_request_str_append_newline (canonical);
kms_request_str_append_hashed (
&request->crypto, canonical, request->payload);

kms_request_str_destroy (normalized);
kms_kv_list_destroy (lst);
kms_request_str_append_newline (canonical);
if (!kms_request_str_append_hashed (
&request->crypto, canonical, request->payload)) {
KMS_ERROR (request, "could not generate hash");
kms_request_str_destroy (canonical);
return NULL;
}

return kms_request_str_detach (canonical);
}
Expand Down
11 changes: 11 additions & 0 deletions kms-message/src/kms_request_str.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,13 @@ kms_request_str_t *
kms_request_str_new (void)
{
kms_request_str_t *s = malloc (sizeof (kms_request_str_t));
KMS_ASSERT (s);

s->len = 0;
s->size = 16;
s->str = malloc (s->size);
KMS_ASSERT (s->str);

s->str[0] = '\0';

return s;
Expand All @@ -64,11 +67,15 @@ kms_request_str_t *
kms_request_str_new_from_chars (const char *chars, ssize_t len)
{
kms_request_str_t *s = malloc (sizeof (kms_request_str_t));
KMS_ASSERT (s);

size_t actual_len;

actual_len = len < 0 ? strlen (chars) : (size_t) len;
s->size = actual_len + 1;
s->str = malloc (s->size);
KMS_ASSERT (s->str);

memcpy (s->str, chars, actual_len);
s->str[actual_len] = '\0';
s->len = actual_len;
Expand All @@ -86,6 +93,8 @@ kms_request_str_wrap (char *chars, ssize_t len)
}

s = malloc (sizeof (kms_request_str_t));
KMS_ASSERT (s);


s->str = chars;
s->len = len < 0 ? strlen (chars) : (size_t) len;
Expand Down Expand Up @@ -148,6 +157,8 @@ kms_request_str_t *
kms_request_str_dup (kms_request_str_t *str)
{
kms_request_str_t *dup = malloc (sizeof (kms_request_str_t));
KMS_ASSERT (dup);


dup->str = strndup (str->str, str->len);
dup->len = str->len;
Expand Down
4 changes: 4 additions & 0 deletions kms-message/src/kms_response_parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ kms_response_parser_t *
kms_response_parser_new (void)
{
kms_response_parser_t *parser = malloc (sizeof (kms_response_parser_t));
KMS_ASSERT (parser);

_parser_init (parser);
return parser;
}
Expand Down Expand Up @@ -72,6 +74,8 @@ static bool
_parse_int_from_view (const char *str, int start, int end, int *result)
{
char *num_str = malloc (end - start + 1);
KMS_ASSERT (num_str);

bool ret;

strncpy (num_str, str + start, end - start);
Expand Down
4 changes: 4 additions & 0 deletions kms-message/test/test_kms_request.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ realloc_buffer (char **buffer, size_t *n, size_t len)
{
if (*buffer == NULL) {
*buffer = malloc (len);
KMS_ASSERT (*buffer);

} else {
*buffer = realloc (*buffer, len);
}
Expand Down Expand Up @@ -203,6 +205,8 @@ read_test (const char *path, const char *suffix)

f_size = (size_t) file_stat.st_size;
str = malloc (f_size + 1);
KMS_ASSERT (str);

memset (str, 0, f_size + 1);

// Windows will convert crlf to lf
Expand Down
8 changes: 8 additions & 0 deletions src/crypto/cng.c
Original file line number Diff line number Diff line change
Expand Up @@ -109,15 +109,21 @@ _crypto_state_init (const _mongocrypt_buffer_t *key,
keyBlob = NULL;

state = bson_malloc0 (sizeof (*state));
BSON_ASSERT (state);

state->key_handle = INVALID_HANDLE_VALUE;

/* Initialize key storage buffer */
state->key_object = bson_malloc0 (_aes256_key_blob_length);
BSON_ASSERT (state->key_object);

state->key_object_length = _aes256_key_blob_length;

/* Allocate temporary buffer for key import */
keyBlobLength = sizeof (BCRYPT_KEY_DATA_BLOB_HEADER) + key->len;
keyBlob = bson_malloc0 (keyBlobLength);
BSON_ASSERT (keyBlob);


blobHeader.dwMagic = BCRYPT_KEY_DATA_BLOB_MAGIC;
blobHeader.dwVersion = BCRYPT_KEY_DATA_BLOB_VERSION1;
Expand All @@ -144,6 +150,8 @@ _crypto_state_init (const _mongocrypt_buffer_t *key,
bson_free (keyBlob);

state->iv = bson_malloc0 (iv->len);
BSON_ASSERT (state->iv);

state->iv_len = iv->len;
memcpy (state->iv, iv->data, iv->len);

Expand Down
2 changes: 2 additions & 0 deletions src/crypto/commoncrypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ _native_crypto_hmac_sha_512 (const _mongocrypt_buffer_t *key,
}

ctx = bson_malloc0 (sizeof (*ctx));
BSON_ASSERT (ctx);


CCHmacInit (ctx, kCCHmacAlgSHA512, key->data, key->len);
CCHmacUpdate (ctx, in->data, in->len);
Expand Down
26 changes: 23 additions & 3 deletions src/mongocrypt-buffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ _make_owned (_mongocrypt_buffer_t *buf)
}
tmp = buf->data;
buf->data = bson_malloc (buf->len);
BSON_ASSERT (buf->data);

memcpy (buf->data, tmp, buf->len);
buf->owned = true;
}
Expand All @@ -57,12 +59,14 @@ _mongocrypt_buffer_resize (_mongocrypt_buffer_t *buf, uint32_t len)
but a fancier implementation could copy over up to 'len'
bytes from the old buffer to the new one. */
if (buf->owned) {
bson_realloc (buf->data, len);
buf->data = bson_realloc (buf->data, len);
buf->len = len;
return;
}

buf->data = bson_malloc (len);
BSON_ASSERT (buf->data);

buf->len = len;
buf->owned = true;
}
Expand Down Expand Up @@ -221,7 +225,13 @@ _mongocrypt_buffer_copy_to (const _mongocrypt_buffer_t *src,
BSON_ASSERT (dst);

_mongocrypt_buffer_cleanup (dst);
if (src->len == 0) {
return;
}

dst->data = bson_malloc ((size_t) src->len);
BSON_ASSERT (dst->data);

memcpy (dst->data, src->data, src->len);
dst->len = src->len;
dst->subtype = src->subtype;
Expand Down Expand Up @@ -294,6 +304,8 @@ _mongocrypt_buffer_to_bson_value (_mongocrypt_buffer_t *plaintext,
le_data_len = BSON_UINT32_TO_LE (data_len);

data = bson_malloc0 (data_len);
BSON_ASSERT (data);

memcpy (data + data_prefix, plaintext->data, plaintext->len);
memcpy (data, &le_data_len, INT32_LEN);
memcpy (data + INT32_LEN, &type, TYPE_LEN);
Expand All @@ -303,11 +315,13 @@ _mongocrypt_buffer_to_bson_value (_mongocrypt_buffer_t *plaintext,
goto fail;
}

if (!bson_validate (&wrapper, 0, NULL)) {
if (!bson_validate (&wrapper, BSON_VALIDATE_NONE, NULL)) {
goto fail;
}

bson_iter_init_find (&iter, &wrapper, "");
if (!bson_iter_init_find (&iter, &wrapper, "")) {
goto fail;
}
bson_value_copy (bson_iter_value (&iter), out);

/* Due to an open libbson bug (CDRIVER-3340), give an empty
Expand Down Expand Up @@ -345,6 +359,8 @@ _mongocrypt_buffer_from_iter (_mongocrypt_buffer_t *plaintext,
plaintext->len =
wrapper.len - offset - NULL_BYTE_LEN; /* the final null byte */
plaintext->data = bson_malloc (plaintext->len);
BSON_ASSERT (plaintext->data);

plaintext->owned = true;
memcpy (plaintext->data, wrapper_data + offset, plaintext->len);

Expand Down Expand Up @@ -402,6 +418,8 @@ _mongocrypt_buffer_copy_from_hex (_mongocrypt_buffer_t *buf, const char *hex)

buf->len = (uint32_t) strlen (hex) / 2;
buf->data = bson_malloc (buf->len);
BSON_ASSERT (buf->data);

buf->owned = true;
for (i = 0; i < buf->len; i++) {
int tmp;
Expand All @@ -426,6 +444,8 @@ char *
_mongocrypt_buffer_to_hex (_mongocrypt_buffer_t *buf)
{
char *hex = bson_malloc0 (buf->len * 2 + 1);
BSON_ASSERT (hex);

char *out = hex;

for (uint32_t i = 0; i < buf->len; i++, out += 2) {
Expand Down
4 changes: 4 additions & 0 deletions src/mongocrypt-cache-key.c
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ _mongocrypt_cache_key_value_new (_mongocrypt_key_doc_t *key_doc,
BSON_ASSERT (decrypted_key_material);

key_value = bson_malloc0 (sizeof (*key_value));
BSON_ASSERT (key_value);

_mongocrypt_buffer_copy_to (decrypted_key_material,
&key_value->decrypted_key_material);

Expand Down Expand Up @@ -155,6 +157,8 @@ _mongocrypt_cache_key_attr_new (_mongocrypt_buffer_t *id,
}

attr = bson_malloc0 (sizeof (*attr));
BSON_ASSERT (attr);

if (id) {
_mongocrypt_buffer_copy_to (id, &attr->id);
}
Expand Down
2 changes: 2 additions & 0 deletions src/mongocrypt-cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ _pair_new (_mongocrypt_cache_t *cache, void *attr)
_mongocrypt_cache_pair_t *pair;

pair = bson_malloc0 (sizeof (_mongocrypt_cache_pair_t));
BSON_ASSERT (pair);

pair->attr = cache->copy_attr (attr);
/* add rest of values. */
pair->next = cache->pair;
Expand Down
4 changes: 4 additions & 0 deletions src/mongocrypt-ciphertext.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ _mongocrypt_serialize_ciphertext (_mongocrypt_ciphertext_t *ciphertext,
offset = 0;
out->len = 1 + ciphertext->key_id.len + 1 + ciphertext->data.len;
out->data = bson_malloc0 (out->len);
BSON_ASSERT (out->data);

out->owned = true;

out->data[offset] = ciphertext->blob_subtype;
Expand Down Expand Up @@ -179,6 +181,8 @@ _mongocrypt_ciphertext_serialize_associated_data (

out->len = 1 + ciphertext->key_id.len + 1;
out->data = bson_malloc (out->len);
BSON_ASSERT (out->data);

out->owned = true;
memcpy (out->data, &ciphertext->blob_subtype, 1);
bytes_written = 1;
Expand Down
Loading

0 comments on commit f602d75

Please sign in to comment.