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

kevinAlbs · kevinAlbs · commit 5c48b5f5dd4f · 2025-03-17T14:49:06.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
@@ -179,7 +179,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
@@ -111,7 +111,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
@@ -114,7 +114,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
@@ -300,7 +300,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
@@ -167,7 +167,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(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(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
@@ -242,9 +242,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(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(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
`@@ -179,7 +179,6 @@ bool mc_FLE2EncryptionPlaceholder_parse(mc_FLE2EncryptionPlaceholder_t *out,`
`179`	`179`	`return true;`
`180`	`180`
`181`	`181`	`fail:`
`182`		`- mc_FLE2EncryptionPlaceholder_cleanup(out);`
`183`	`182`	`return false;`
`184`	`183`	`}`
`185`	`184`
Original file line number	Diff line number	Diff line change
`@@ -111,7 +111,6 @@ bool mc_FLE2FindEqualityPayloadV2_parse(mc_FLE2FindEqualityPayloadV2_t *out,`
`111`	`111`
`112`	`112`	`return true;`
`113`	`113`	`fail:`
`114`		`- mc_FLE2FindEqualityPayloadV2_cleanup(out);`
`115`	`114`	`return false;`
`116`	`115`	`}`
`117`	`116`
Original file line number	Diff line number	Diff line change
`@@ -114,7 +114,6 @@ bool mc_FLE2FindEqualityPayload_parse(mc_FLE2FindEqualityPayload_t *out,`
`114`	`114`
`115`	`115`	`return true;`
`116`	`116`	`fail:`
`117`		`- mc_FLE2FindEqualityPayload_cleanup(out);`
`118`	`117`	`return false;`
`119`	`118`	`}`
`120`	`119`
Original file line number	Diff line number	Diff line change
`@@ -300,7 +300,6 @@ bool mc_FLE2InsertUpdatePayloadV2_parse(mc_FLE2InsertUpdatePayloadV2_t *out,`
`300`	`300`
`301`	`301`	`return true;`
`302`	`302`	`fail:`
`303`		`- mc_FLE2InsertUpdatePayloadV2_cleanup(out);`
`304`	`303`	`return false;`
`305`	`304`	`}`
`306`	`305`
Original file line number	Diff line number	Diff line change
`@@ -167,7 +167,6 @@ bool mc_FLE2InsertUpdatePayload_parse(mc_FLE2InsertUpdatePayload_t *out,`
`167`	`167`
`168`	`168`	`return true;`
`169`	`169`	`fail:`
`170`		`- mc_FLE2InsertUpdatePayload_cleanup(out);`
`171`	`170`	`return false;`
`172`	`171`	`}`
`173`	`172`