[MONGOCRYPT-559] _fle2_mongo_op_markings and _create_markings_cmd_bson are double-initializing bson_t’s Created: 10/Mar/23  Updated: 27/Apr/23  Resolved: 27/Apr/23

Status: Closed
Project: Libmongocrypt
Component/s: None
Affects Version/s: None
Fix Version/s: 1.8.0

Type: Bug Priority: Unknown
Reporter: Zachary Espiritu Assignee: Kyle Kloberdanz
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Binding Changes: Not Needed

 Description   

In mongocrypt-ctx-encrypt.c, a Valgrind memory leak can occur when the libbson BSON_MEMCHECK compile flag is set in two possible ways.

  1. bson_init and bson_init_static on same bson_t instance
    • In _fle2_mongo_op_markings, bson_t cmd_bson and encrypted_field_config_bson is initialized on lines 627–628 (as of commit 2ec9c3). Then, cmd_bson and encrypted_field_config_bson are passed as the bson argument to _mongocrypt_buffer_to_bson, which double-initializes the input bson with bson_init_static.
      • This can be fixed by removing BSON_INITIALIZER from the declaration of cmd_bson and encrypted_field_config_bson.
    • Similarly, in _create_markings_cmd_bson, bson_t bson_view is initialized on line 696 and is passed to _mongocrypt_buffer_to_bson, which double-initializes bson_view.
      • This can be fixed by removing BSON_INITIALIZER from the declaration of bson_view.
  2. bson_init twice on same bson_t
    • Another double-init issue can occur as callers of _create_markings_cmd_bson pass their out argument as already-initialized (see this example). Then, there are two possible ways the out argument can be initialized again within the scope of _create_markings_cmd_bson:
      • For an FLE1 command, it calls bson_init on line 706
      • For an FLE2 command, the processing of the command gets passed to _fle2_mongo_op_markings which calls bson_init on out on line 654
    • To fix this issue, the calls to bson_init on 654 and 706 can be removed since the callers of _create_markings_cmd_bson are already initializing the input bson_t structure.

To reproduce the memory leak, use

mkdir cmake-build && cd cmake-build
cmake ../
make
valgrind --leak-check=full ./test-mongocrypt 



 Comments   
Comment by Githook User [ 27/Apr/23 ]

Author:

{'name': 'Kyle Kloberdanz', 'email': 'kyle.kloberdanz@mongodb.com', 'username': 'kkloberdanz'}

Message: Improve bson_t initialization behavior (#629)

MONGOCRYPT-558
MONGOCRYPT-559
MONGOCRYPT-560

This addresses the issues brought up in the above tickets. We will not be removing calls to `BSON_INITIALIZER`, but we are addressing the other issues uncovered in these tickets.

Co-authored-by: Kevin Albertson <kevin.albertson@10gen.com>
Branch: master
https://github.com/mongodb/libmongocrypt/commit/ec15461d69cd740d9b8b5f195fc920a2347005dd

Comment by Kyle Kloberdanz [ 25/Apr/23 ]

Hello Zachary, thank you for reporting this.

I've made sure to clean up several of these issues, and I'll have a PR with the fixes ready soon. However, we prefer to not remove these calls to BSON_INITIALIZER. Since we do not use BSON_MEMCHECK, initializing a bson object with BSON_INITIALIZER doesn't cause us an issue, and it makes the code more robust since it avoids dealing with uninitialized objects.

Please also see Kevin's response on another ticket.

Generated at Thu Feb 08 09:08:57 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.