[MONGOCRYPT-554] _fle2_finalize is missing calls to bson_destroy in an error case / double-initializing bson_t Created: 09/Mar/23  Updated: 28/Oct/23  Resolved: 14/Mar/23

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

Type: Bug Priority: Unknown
Reporter: Zachary Espiritu Assignee: Kevin Albertson
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Binding Changes: Not Needed

 Description   

In _fle2_finalize in mongocrypt-ctx-encrypt.c, there are two potential  memory leak errors that occur:

  1. The error case on line 1663 (as of commit 2ec9c3) should bson_destroy the converted and deleteTokens instances but is not currently doing so. (It also looks like the error case should return _mongocrypt_ctx_fail (ctx) instead of false in this case.)
  2. bson_copy_to on line 1617 (as of commit 2ec9c3) requires that the destination argument is an uninitialized bson_t (documentation) but line 1616 calls bson_init on the destination converted.

To reproduce, use

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



 Comments   
Comment by Githook User [ 20/Mar/23 ]

Author:

{'name': 'Kevin Albertson', 'email': 'kevin.albertson@mongodb.com', 'username': 'kevinAlbs'}

Message: MONGOCRYPT-554 fix cleanup in `_fle2_finalize` error case (#595)

  • add missing destroy and context fail on error
  • remove unnecessary `bson_init`

`bson_copy_to` initializes the destination
Branch: shreyaskalyan/MONGOCRYPT-539
https://github.com/mongodb/libmongocrypt/commit/c89c984e0d1b2cf4b8735433d060f664a5535a36

Comment by Githook User [ 14/Mar/23 ]

Author:

{'name': 'Kevin Albertson', 'email': 'kevin.albertson@mongodb.com', 'username': 'kevinAlbs'}

Message: MONGOCRYPT-554 fix cleanup in `_fle2_finalize` error case (#595)

  • add missing destroy and context fail on error
  • remove unnecessary `bson_init`

`bson_copy_to` initializes the destination
Branch: r1.7
https://github.com/mongodb/libmongocrypt/commit/0caa1d3249d85050d0e3422d602e630aca574e0b

Comment by Githook User [ 14/Mar/23 ]

Author:

{'name': 'Kevin Albertson', 'email': 'kevin.albertson@mongodb.com', 'username': 'kevinAlbs'}

Message: MONGOCRYPT-554 fix cleanup in `_fle2_finalize` error case (#595)

  • add missing destroy and context fail on error
  • remove unnecessary `bson_init`

`bson_copy_to` initializes the destination
Branch: master
https://github.com/mongodb/libmongocrypt/commit/c89c984e0d1b2cf4b8735433d060f664a5535a36

Comment by Kevin Albertson [ 14/Mar/23 ]

Thank you for the report.

In _fle2_finalize in mongocrypt-ctx-encrypt.c, there are two potential  memory leak errors that occur:

  1. The error case on line 1663 (as of commit 2ec9c3) should bson_destroy the converted and deleteTokens instances but is not currently doing so. (It also looks like the error case should return _mongocrypt_ctx_fail (ctx) instead of false in this case.)

Good catch. The error path does not appear to be exercised by tests.

  1. bson_copy_to on line 1617 (as of commit 2ec9c3) requires that the destination argument is an uninitialized bson_t (documentation) but line 1616 calls bson_init on the destination converted.

Was this discovered by building the C driver with -DBSON_MEMCHECK? If yes, the C driver (and libmongocrypt) no longer test building with -DBSON_MEMCHECK. Testing with BSON_MEMCHECK was removed in this commit. Documented use of BSON_MEMCHECK was removed in this commit. We do not intend to the check with BSON_MEMCHECK. BSON_MEMCHECK added development difficulty (breaking ABI of bson_t) and rarely discovered legitimate leaks.

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