MONGOCRYPT-865 replace sprintf with safer alternatives (#1127)

* remove unused `_mongocrypt_new_json_string_from_binary`

* remove unused `tmp_buf`

* replace calls of `sprintf` with `bson_snprintf` or `snprintf`

* check return values of `snprintf` and `bson_snprintf`

* fix bound check of `bson_snprintf`

The return value does not include the NULL byte. To ensure no truncation occurs, the return value must be strictly less than the buffer size.

Author: kevinAlbs

kms-message/src/hexlify.c

@@ -35,7 +35,8 @@ hexlify (const uint8_t *buf, size_t len)
    size_t i;
 
    for (i = 0; i < len; i++) {
-      p += sprintf (p, "%02x", buf[i]);
+      KMS_ASSERT (2 == snprintf (p, 3, "%02x", buf[i]));
+      p += 2;
    }
 
    *p = '\0';

kms-message/src/kms_request_str.c

@@ -303,7 +303,7 @@ kms_request_str_append_escaped (kms_request_str_t *str,
          ++out;
          ++str->len;
       } else {
-         sprintf ((char *) out, "%%%02X", *in);
+         KMS_ASSERT (3 == snprintf ((char *) out, 4, "%%%02X", *in));
          out += 3;
          str->len += 3;
       }

kms-message/test/test_kms_request.c

@@ -99,7 +99,8 @@ test_file_path (const char *path, const char *suffix)
    char *r;
    char *test_name = last_segment (path);
    char file_path[PATH_MAX];
-   snprintf (file_path, PATH_MAX, "%s/%s.%s", path, test_name, suffix);
+   int ret = snprintf (file_path, PATH_MAX, "%s/%s.%s", path, test_name, suffix);
+   KMS_ASSERT (ret > 0 && ret < PATH_MAX);
    r = strdup (file_path);
    free (test_name);
    return r;

src/mc-schema-broker.c

@@ -185,7 +185,7 @@ bool mc_schema_broker_append_listCollections_filter(const mc_schema_broker_t *sb
 
             char idx_str[32];
             int ret = bson_snprintf(idx_str, sizeof idx_str, "%zu", idx);
-            BSON_ASSERT(ret > 0 && ret <= (int)sizeof idx_str);
+            BSON_ASSERT(ret > 0 && ret < (int)sizeof idx_str);
 
             TRY_BSON_OR(BSON_APPEND_UTF8(&in_array, idx_str, se->coll)) {
                 return false;

src/mongocrypt-buffer.c

@@ -462,7 +462,7 @@ char *_mongocrypt_buffer_to_hex(_mongocrypt_buffer_t *buf) {
     char *out = hex;
 
     for (uint32_t i = 0; i < buf->len; i++, out += 2) {
-        sprintf(out, "%02X", buf->data[i]);
+        BSON_ASSERT(2 == bson_snprintf(out, 3, "%02X", buf->data[i]));
     }
     return hex;
 }

src/mongocrypt-private.h

@@ -163,8 +163,6 @@ bool _mongocrypt_validate_and_copy_string(const char *in, int32_t in_len, char *
 
 char *_mongocrypt_new_string_from_bytes(const void *in, int len);
 
-char *_mongocrypt_new_json_string_from_binary(mongocrypt_binary_t *binary);
-
 /* _mongocrypt_needs_credentials returns true if @crypt was configured to
  * request credentials for any KMS provider. */
 bool _mongocrypt_needs_credentials(mongocrypt_t *crypt);

src/mongocrypt.c

@@ -75,31 +75,11 @@ const char *tmp_json(const bson_t *bson) {
 
     memset(storage, 0, 1024);
     json = bson_as_canonical_extended_json(bson, NULL);
-    bson_snprintf(storage, sizeof(storage), "%s", json);
+    BSON_ASSERT(0 < bson_snprintf(storage, sizeof(storage), "%s", json)); // Truncation OK.
     bson_free(json);
     return (const char *)storage;
 }
 
-const char *tmp_buf(const _mongocrypt_buffer_t *buf) {
-    static char storage[1024];
-    size_t i, n;
-
-    BSON_ASSERT_PARAM(buf);
-
-    memset(storage, 0, 1024);
-    /* capped at two characters per byte, minus 1 for trailing \0 */
-    n = sizeof(storage) / 2 - 1;
-    if (buf->len < n) {
-        n = buf->len;
-    }
-
-    for (i = 0; i < n; i++) {
-        bson_snprintf(storage + (i * 2), 3, "%02x", buf->data[i]);
-    }
-
-    return (const char *)storage;
-}
-
 static void _mongocrypt_do_init(void) {
     (void)kms_message_init();
     _native_crypto_init();
@@ -218,47 +198,6 @@ bool mongocrypt_setopt_key_expiration(mongocrypt_t *crypt, uint64_t cache_expira
     return true;
 }
 
-char *_mongocrypt_new_string_from_bytes(const void *in, int len) {
-    const int max_bytes = 100;
-    const int chars_per_byte = 2;
-    int out_size = max_bytes * chars_per_byte;
-    const unsigned char *src = in;
-    char *out;
-    char *ret;
-
-    out_size += len > max_bytes ? (int)sizeof("...") : 1 /* for null */;
-    out = bson_malloc0((size_t)out_size);
-    BSON_ASSERT(out);
-
-    ret = out;
-
-    for (int i = 0; i < len && i < max_bytes; i++, out += chars_per_byte) {
-        sprintf(out, "%02X", src[i]);
-    }
-
-    sprintf(out, (len > max_bytes) ? "..." : "");
-    return ret;
-}
-
-char *_mongocrypt_new_json_string_from_binary(mongocrypt_binary_t *binary) {
-    bson_t bson;
-    uint32_t len;
-
-    BSON_ASSERT_PARAM(binary);
-
-    if (!_mongocrypt_binary_to_bson(binary, &bson) || !bson_validate(&bson, BSON_VALIDATE_NONE, NULL)) {
-        char *hex;
-        char *full_str;
-
-        BSON_ASSERT(binary->len <= (uint32_t)INT_MAX);
-        hex = _mongocrypt_new_string_from_bytes(binary->data, (int)binary->len);
-        full_str = bson_strdup_printf("(malformed) %s", hex);
-        bson_free(hex);
-        return full_str;
-    }
-    return bson_as_canonical_extended_json(&bson, (size_t *)&len);
-}
-
 bool mongocrypt_setopt_schema_map(mongocrypt_t *crypt, mongocrypt_binary_t *schema_map) {
     ASSERT_MONGOCRYPT_PARAM_UNINIT(crypt);
 

test/test-mongocrypt-assert-match-bson.c

@@ -230,7 +230,7 @@ static void match_err(match_ctx_t *ctx, const char *fmt, ...) {
     formatted = bson_strdupv_printf(fmt, args);
     va_end(args);
 
-    bson_snprintf(ctx->errmsg, sizeof ctx->errmsg, "%s: %s", ctx->path, formatted);
+    BSON_ASSERT(0 < bson_snprintf(ctx->errmsg, sizeof ctx->errmsg, "%s: %s", ctx->path, formatted)); // Truncation OK.
 
     bson_free(formatted);
 }
@@ -245,9 +245,9 @@ static void derive(match_ctx_t *ctx, match_ctx_t *derived, const char *key) {
     derived->strict_numeric_types = ctx->strict_numeric_types;
 
     if (strlen(ctx->path) > 0) {
-        bson_snprintf(derived->path, sizeof derived->path, "%s.%s", ctx->path, key);
+        BSON_ASSERT(0 < bson_snprintf(derived->path, sizeof derived->path, "%s.%s", ctx->path, key)); // Truncation OK.
     } else {
-        bson_snprintf(derived->path, sizeof derived->path, "%s", key);
+        BSON_ASSERT(0 < bson_snprintf(derived->path, sizeof derived->path, "%s", key)); // Truncation OK.
     }
     derived->retain_dots_in_keys = ctx->retain_dots_in_keys;
     derived->allow_placeholders = ctx->allow_placeholders;

test/test-mongocrypt-buffer.c

@@ -30,7 +30,7 @@ static void _get_bytes(const void *in, char *out, int len) {
     char *dest = out;
 
     for (int i = 0; i < len; i++, dest += 3) {
-        sprintf(dest, "%02X ", src[i]);
+        ASSERT(3 == bson_snprintf(dest, 4, "%02X ", src[i]));
     }
     dest[-1] = '\0';
 }

test/test-mongocrypt-marking.c

@@ -2036,8 +2036,10 @@ static void test_ciphertext_len_steps_fle2_text_search(_mongocrypt_tester_t *tes
         char *v = bson_malloc0(str_len + 1);
         memset(v, 'a', str_len);
         size_t bufsize = snprintf(NULL, 0, MARKING_JSON_FORMAT, v) + 1;
+        ASSERT(bufsize > 0);
         char *markingJSON = bson_malloc(bufsize);
-        sprintf(markingJSON, MARKING_JSON_FORMAT, v);
+        int ret = bson_snprintf(markingJSON, bufsize, MARKING_JSON_FORMAT, v);
+        BSON_ASSERT(0 < ret && ret < (int)bufsize);
         bson_t *marking_bson = TMP_BSON_STR(markingJSON);
 
         _mongocrypt_ciphertext_t ciphertext;