MONGOCRYPT-794 fix possible double free on parse error (#988)

kevinAlbs · web-flow · commit 27452ca30718 · 2025-03-17T14:47:20.000-04:00
* test and fix `mc_FLE2FindEqualityPayloadV2_t`

* fix unused `mc_FLE2FindEqualityPayload_t`

Payload is unused but still fixed. The `mc_FLE2FindEqualityPayload_t` may be safe to remove

* test and fix `mc_FLE2EncryptionPlaceholder_t`

* test and fix `mc_FLE2InsertUpdatePayloadV2_t`

* test and fix `mc_FLE2InsertUpdatePayload_t`
diff --git a/src/mc-fle2-encryption-placeholder.c b/src/mc-fle2-encryption-placeholder.c
@@ -184,7 +184,6 @@ bool mc_FLE2EncryptionPlaceholder_parse(mc_FLE2EncryptionPlaceholder_t *out,
     return true;
 
 fail:
-    mc_FLE2EncryptionPlaceholder_cleanup(out);
     return false;
 }
 
diff --git a/src/mc-fle2-find-equality-payload-v2.c b/src/mc-fle2-find-equality-payload-v2.c
@@ -114,7 +114,6 @@ bool mc_FLE2FindEqualityPayloadV2_parse(mc_FLE2FindEqualityPayloadV2_t *out,
 
     return true;
 fail:
-    mc_FLE2FindEqualityPayloadV2_cleanup(out);
     return false;
 }
 
diff --git a/src/mc-fle2-find-equality-payload.c b/src/mc-fle2-find-equality-payload.c
@@ -117,7 +117,6 @@ bool mc_FLE2FindEqualityPayload_parse(mc_FLE2FindEqualityPayload_t *out,
 
     return true;
 fail:
-    mc_FLE2FindEqualityPayload_cleanup(out);
     return false;
 }
 
diff --git a/src/mc-fle2-insert-update-payload-v2.c b/src/mc-fle2-insert-update-payload-v2.c
@@ -301,7 +301,6 @@ bool mc_FLE2InsertUpdatePayloadV2_parse(mc_FLE2InsertUpdatePayloadV2_t *out,
 
     return true;
 fail:
-    mc_FLE2InsertUpdatePayloadV2_cleanup(out);
     return false;
 }
 
diff --git a/src/mc-fle2-insert-update-payload.c b/src/mc-fle2-insert-update-payload.c
@@ -168,7 +168,6 @@ bool mc_FLE2InsertUpdatePayload_parse(mc_FLE2InsertUpdatePayload_t *out,
 
     return true;
 fail:
-    mc_FLE2InsertUpdatePayload_cleanup(out);
     return false;
 }
 
diff --git a/test/test-mc-fle2-encryption-placeholder.c b/test/test-mc-fle2-encryption-placeholder.c
@@ -440,8 +440,29 @@ static void _test_FLE2EncryptionPlaceholder_textSearch_parse(_mongocrypt_tester_
     }
 }
 
+static void _test_FLE2EncryptionPlaceholder_parse_errors(_mongocrypt_tester_t *tester) {
+    bson_t *input_bson = TMP_BSON_STR(BSON_STR({
+        "t" : {"$numberInt" : "1"},
+        "a" : {"$numberInt" : "1"},
+        "ki" : {"$binary" : {"base64" : "EjRWeBI0mHYSNBI0VniQEg==", "subType" : "04"}},
+        "ku" : {"$binary" : {"base64" : "q83vqxI0mHYSNBI0VniQEg==", "subType" : "04"}},
+        "v" : "foobar",
+        "cm" : "wrong type!"
+    }));
+
+    mc_FLE2EncryptionPlaceholder_t payload;
+    mc_FLE2EncryptionPlaceholder_init(&payload);
+    mongocrypt_status_t *status = mongocrypt_status_new();
+    ASSERT_FAILS_STATUS(mc_FLE2EncryptionPlaceholder_parse(&payload, input_bson, status),
+                        status,
+                        "'cm' must be an int64");
+    mc_FLE2EncryptionPlaceholder_cleanup(&payload);
+    mongocrypt_status_destroy(status);
+}
+
 void _mongocrypt_tester_install_fle2_encryption_placeholder(_mongocrypt_tester_t *tester) {
     INSTALL_TEST(_test_FLE2EncryptionPlaceholder_parse);
     INSTALL_TEST(_test_FLE2EncryptionPlaceholder_range_parse);
     INSTALL_TEST(_test_FLE2EncryptionPlaceholder_textSearch_parse);
+    INSTALL_TEST(_test_FLE2EncryptionPlaceholder_parse_errors);
 }
diff --git a/test/test-mc-fle2-find-equality-payload-v2.c b/test/test-mc-fle2-find-equality-payload-v2.c
@@ -73,6 +73,25 @@ static void _test_FLE2FindEqualityPayloadV2_roundtrip(_mongocrypt_tester_t *test
 
 #undef TEST_FIND_EQ_PAYLOAD_HEX_V2
 
+static void _test_FLE2FindEqualityPayloadV2_errors(_mongocrypt_tester_t *tester) {
+    bson_t *input_bson = TMP_BSON_STR(BSON_STR({
+        "d" : {"$binary" : {"base64" : "AAAA", "subType" : "00"}},
+        "s" : {"$binary" : {"base64" : "AAAA", "subType" : "00"}},
+        "l" : {"$binary" : {"base64" : "AAAA", "subType" : "00"}},
+        "cm" : "wrong type!"
+    }));
+
+    mc_FLE2FindEqualityPayloadV2_t payload;
+    mc_FLE2FindEqualityPayloadV2_init(&payload);
+    mongocrypt_status_t *status = mongocrypt_status_new();
+    ASSERT_FAILS_STATUS(mc_FLE2FindEqualityPayloadV2_parse(&payload, input_bson, status),
+                        status,
+                        "Field 'cm' expected to hold an int64");
+    mc_FLE2FindEqualityPayloadV2_cleanup(&payload);
+    mongocrypt_status_destroy(status);
+}
+
 void _mongocrypt_tester_install_fle2_payload_find_equality_v2(_mongocrypt_tester_t *tester) {
     INSTALL_TEST(_test_FLE2FindEqualityPayloadV2_roundtrip);
+    INSTALL_TEST(_test_FLE2FindEqualityPayloadV2_errors);
 }
diff --git a/test/test-mc-fle2-payload-iup-v2.c b/test/test-mc-fle2-payload-iup-v2.c
@@ -227,9 +227,31 @@ static void _test_mc_FLE2InsertUpdatePayloadV2_parses_crypto_params(_mongocrypt_
     mc_FLE2InsertUpdatePayloadV2_cleanup(&got);
 }
 
+static void _test_mc_FLE2InsertUpdatePayloadV2_parse_errors(_mongocrypt_tester_t *tester) {
+    bson_t *input_bson = TMP_BSON_STR(BSON_STR({
+        "d" : {"$binary" : {"base64" : "AAAA", "subType" : "00"}}, //
+        "t" : "wrong type!"
+    }));
+    _mongocrypt_buffer_t input_buf;
+    _mongocrypt_buffer_init_size(&input_buf, 1 + input_bson->len);
+    input_buf.data[0] = (uint8_t)MC_SUBTYPE_FLE2InsertUpdatePayloadV2;
+    memcpy(input_buf.data + 1, bson_get_data(input_bson), input_bson->len);
+
+    mc_FLE2InsertUpdatePayloadV2_t payload;
+    mc_FLE2InsertUpdatePayloadV2_init(&payload);
+    mongocrypt_status_t *status = mongocrypt_status_new();
+    ASSERT_FAILS_STATUS(mc_FLE2InsertUpdatePayloadV2_parse(&payload, &input_buf, status),
+                        status,
+                        "Field 't' expected to hold an int32");
+    mc_FLE2InsertUpdatePayloadV2_cleanup(&payload);
+    mongocrypt_status_destroy(status);
+    _mongocrypt_buffer_cleanup(&input_buf);
+}
+
 void _mongocrypt_tester_install_fle2_payload_iup_v2(_mongocrypt_tester_t *tester) {
     INSTALL_TEST(_test_FLE2InsertUpdatePayloadV2_parse);
     INSTALL_TEST(_test_mc_FLE2InsertUpdatePayloadV2_decrypt);
     INSTALL_TEST(_test_mc_FLE2InsertUpdatePayloadV2_includes_crypto_params);
     INSTALL_TEST(_test_mc_FLE2InsertUpdatePayloadV2_parses_crypto_params);
+    INSTALL_TEST(_test_mc_FLE2InsertUpdatePayloadV2_parse_errors);
 }
diff --git a/test/test-mc-fle2-payload-iup.c b/test/test-mc-fle2-payload-iup.c
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#include "mc-fle-blob-subtype-private.h"
 #include "mc-fle2-insert-update-payload-private.h"
 #include "test-mongocrypt.h"
 
@@ -160,7 +161,29 @@ static void test_FLE2InsertUpdatePayload_decrypt(_mongocrypt_tester_t *tester) {
 
 #undef TEST_IUP_HEX
 
+static void test_FLE2InsertUpdatePayload_parse_errors(_mongocrypt_tester_t *tester) {
+    bson_t *input_bson = TMP_BSON_STR(BSON_STR({
+        "d" : {"$binary" : {"base64" : "AAAA", "subType" : "00"}}, //
+        "t" : "wrong type!"
+    }));
+    _mongocrypt_buffer_t input_buf;
+    _mongocrypt_buffer_init_size(&input_buf, 1 + input_bson->len);
+    input_buf.data[0] = (uint8_t)MC_SUBTYPE_FLE2InsertUpdatePayload;
+    memcpy(input_buf.data + 1, bson_get_data(input_bson), input_bson->len);
+
+    mc_FLE2InsertUpdatePayload_t payload;
+    mc_FLE2InsertUpdatePayload_init(&payload);
+    mongocrypt_status_t *status = mongocrypt_status_new();
+    ASSERT_FAILS_STATUS(mc_FLE2InsertUpdatePayload_parse(&payload, &input_buf, status),
+                        status,
+                        "Field 't' expected to hold an int32");
+    mc_FLE2InsertUpdatePayload_cleanup(&payload);
+    mongocrypt_status_destroy(status);
+    _mongocrypt_buffer_cleanup(&input_buf);
+}
+
 void _mongocrypt_tester_install_fle2_payload_iup(_mongocrypt_tester_t *tester) {
     INSTALL_TEST(test_FLE2InsertUpdatePayload_parse);
     INSTALL_TEST(test_FLE2InsertUpdatePayload_decrypt);
+    INSTALL_TEST(test_FLE2InsertUpdatePayload_parse_errors);
 }

Original file line number	Diff line number	Diff line change
`@@ -184,7 +184,6 @@ bool mc_FLE2EncryptionPlaceholder_parse(mc_FLE2EncryptionPlaceholder_t *out,`
`184`	`184`	`return true;`
`185`	`185`
`186`	`186`	`fail:`
`187`		`- mc_FLE2EncryptionPlaceholder_cleanup(out);`
`188`	`187`	`return false;`
`189`	`188`	`}`
`190`	`189`
Original file line number	Diff line number	Diff line change
`@@ -114,7 +114,6 @@ bool mc_FLE2FindEqualityPayloadV2_parse(mc_FLE2FindEqualityPayloadV2_t *out,`
`114`	`114`
`115`	`115`	`return true;`
`116`	`116`	`fail:`
`117`		`- mc_FLE2FindEqualityPayloadV2_cleanup(out);`
`118`	`117`	`return false;`
`119`	`118`	`}`
`120`	`119`
Original file line number	Diff line number	Diff line change
`@@ -117,7 +117,6 @@ bool mc_FLE2FindEqualityPayload_parse(mc_FLE2FindEqualityPayload_t *out,`
`117`	`117`
`118`	`118`	`return true;`
`119`	`119`	`fail:`
`120`		`- mc_FLE2FindEqualityPayload_cleanup(out);`
`121`	`120`	`return false;`
`122`	`121`	`}`
`123`	`122`
Original file line number	Diff line number	Diff line change
`@@ -301,7 +301,6 @@ bool mc_FLE2InsertUpdatePayloadV2_parse(mc_FLE2InsertUpdatePayloadV2_t *out,`
`301`	`301`
`302`	`302`	`return true;`
`303`	`303`	`fail:`
`304`		`- mc_FLE2InsertUpdatePayloadV2_cleanup(out);`
`305`	`304`	`return false;`
`306`	`305`	`}`
`307`	`306`
Original file line number	Diff line number	Diff line change
`@@ -168,7 +168,6 @@ bool mc_FLE2InsertUpdatePayload_parse(mc_FLE2InsertUpdatePayload_t *out,`
`168`	`168`
`169`	`169`	`return true;`
`170`	`170`	`fail:`
`171`		`- mc_FLE2InsertUpdatePayload_cleanup(out);`
`172`	`171`	`return false;`
`173`	`172`	`}`
`174`	`173`