mongo-c-driver
mongo-c-driver copied to clipboard
CDRIVER-5732 Replace _bson_append with BSON_APPEND_BYTES
Summary
Resolves CDRIVER-5732. Verified by this patch.
Replaces the painfully-difficult-to-maintain _bson_append
internal variadic function with a new macro-based pattern to avoid a wide range of issues concerning integer arithmetic validation and bounds checks.
These changes also address 49 GCC and 51 Clang unique warnings for -Wimplicit-int-conversion
and -Wsign-conversion
.
Details
Using bson_append_utf8
as a representative example, replaces the old pattern of invoking _bson_append
:
// Abundance of magic numbers.
// Formatted code makes length-value pairing difficult to read.
// Potential signed integer overflow in computation of n_bytes argument.
// Significant va_args parsing complexity and internal arithmetic checks.
return _bson_append (bson,
6,
(1 + key_length + 1 + 4 + length + 1), // n_bytes
1,
&type,
key_length,
key,
1,
&gZero,
4,
&length_le,
length,
value,
1, &gZero);
with a new macro-based approach:
// Local state.
BSON_APPEND_BYTES_LIST_DECLARE (args);
// Each additional argument updates the local state.
// No magic numbers or manual arithmetic in usage code.
// Bounds and arithmetic are checked immediately per argument.
BSON_APPEND_BYTES_ADD_ARGUMENT (bson, args, &type, sizeof (type));
BSON_APPEND_BYTES_ADD_CHECKED_STRING (bson, args, key, key_length);
BSON_APPEND_BYTES_ADD_ARGUMENT (bson, args, &gZero, sizeof (gZero));
BSON_APPEND_BYTES_ADD_ARGUMENT (bson, args, &ulength_arg, sizeof (ulength_arg));
BSON_APPEND_BYTES_ADD_ARGUMENT (bson, args, value, ulength);
BSON_APPEND_BYTES_ADD_ARGUMENT (bson, args, &gZero, sizeof (gZero));
// BSON object is only modified after passing all bounds checks above.
BSON_APPEND_BYTES_APPLY_ARGUMENTS (bson, args);
return true;
append_failure:
return false; // For cleanup opportunity on failure.
The local state constitutes an array of stack-allocated objects that contain the data+length pairs to be appended to the BSON document after all bounds checks have completed. This stack-allocated array fulfills the role of the variadic arguments of the old _bson_append
function. However, unlike with _bson_append
, bounds checks are computed immediately and inline at the call site with early return behavior on failure (via goto append_failure;
to permit cleanup routines). This greatly simplifies error handling and arithmetic computation and improves readability while retaining append-only-on-success behavior (no partial writes).
Additionally audited all computation of lengths and strlen
results in associated functions by adding checks to ensure uint32_t
representability as well as avoid any potential for signed integer overflow.