[CDRIVER-6017] BSON Validation Refactor (#2026) (#2032)

* New BSON validation routine rewrite

The new `bson_validate` implementation does not
make use of the error-prone `bson_visit` APIs. Instead, it is written
as a simple recursive validator. The new validator respects requests
for UTF-8 validation properly.

* Stop validating at 1000 depth, preventing stack overflow
* Replace most BSON validation tests with generated ones

The existing test cases used BSON files, and didn't have
any commentary on what they were actually testing. New test cases are
generated from a Python shorthand and contain the tested bytes inline,
with a distinct test case for each actual validation scenario.

* Disable UTF-8 validation by default on CRUD APIs
* Document and tweak the value of BSON_VALIDATE_CORRUPT
* Add test cases related to the overlong null encoding
* Tweak JS scope validation to permit more obj keys
* Add a NEWS entry for validation changes.
* Allow `-private.h` headers to not include the prelude header

---------

Co-authored-by: Kevin Albertson <[email protected]>
Co-authored-by: Ezra Chung <[email protected]>

Author: 3 people

.evergreen/scripts/check-preludes.py

@@ -35,7 +35,7 @@
             MONGOC_PREFIX / "mongoc-prelude.h",
             MONGOC_PREFIX / "mongoc.h",
         ],
-        "include": '#include <mongoc/mongoc-prelude.h>',
+        "include": "#include <mongoc/mongoc-prelude.h>",
     },
     {
         "name": "libbson",
@@ -50,7 +50,7 @@
         "name": "common",
         "headers": list(COMMON_PREFIX.glob("*.h")),
         "exclusions": [COMMON_PREFIX / "common-prelude.h"],
-        "include": '#include <common-prelude.h>',
+        "include": "#include <common-prelude.h>",
     },
 ]
 
@@ -59,7 +59,7 @@
     print(f"Checking headers for {NAME}")
     assert len(check["headers"]) > 0
     for header in check["headers"]:
-        if header in check["exclusions"]:
+        if header in check["exclusions"] or header.name.endswith("-private.h"):
             continue
         lines = Path(header).read_text(encoding="utf-8").splitlines()
         if check["include"] not in lines:

src/libbson/NEWS

@@ -1,3 +1,17 @@
+Unreleased
+==========
+
+Fixes:
+
+* Various fixes have been applied to the `bson_validate` family of functions,
+  with some minor behavioral changes.
+  * Previously accepted invalid UTF-8 will be rejected when `BSON_VALIDATE_UTF8`
+    is specified.
+  * The scope document in a deprecated "code with scope" element is now
+    validated with a fixed set of rules and is treated as an opaque JavaScript
+    object.
+  * A document nesting limit is now enforced during validation.
+
 libbson 2.0.1
 =============
 

src/libbson/doc/bson_validate_flags_t.rst

@@ -19,6 +19,7 @@ Synopsis
     BSON_VALIDATE_DOT_KEYS = (1 << 2),
     BSON_VALIDATE_UTF8_ALLOW_NULL = (1 << 3),
     BSON_VALIDATE_EMPTY_KEYS = (1 << 4),
+    BSON_VALIDATE_CORRUPT = (1 << 5),
   } bson_validate_flags_t;
 
 Description
@@ -40,6 +41,8 @@ Each defined flag aside from ``BSON_VALIDATE_NONE`` describes an optional valida
 * ``BSON_VALIDATE_DOLLAR_KEYS`` Prohibit keys that start with ``$`` outside of a "DBRef" subdocument.
 * ``BSON_VALIDATE_DOT_KEYS`` Prohibit keys that contain ``.`` anywhere in the string.
 * ``BSON_VALIDATE_EMPTY_KEYS`` Prohibit zero-length keys.
+* ``BSON_VALIDATE_CORRUPT`` is not a control flag, but is used as an error code
+  when a validation routine encounters corrupt BSON data.
 
 .. seealso::
 

src/libbson/src/bson/bson-types.h

@@ -185,25 +185,54 @@ typedef struct {
 
 
 /**
- * bson_validate_flags_t:
+ * @brief Flags and error codes for BSON validation functions.
  *
- * This enumeration is used for validation of BSON documents. It allows
- * selective control on what you wish to validate.
+ * Pass these flags bits to control the behavior of the `bson_validate` family
+ * of functions.
  *
- * %BSON_VALIDATE_NONE: No additional validation occurs.
- * %BSON_VALIDATE_UTF8: Check that strings are valid UTF-8.
- * %BSON_VALIDATE_DOLLAR_KEYS: Check that keys do not start with $.
- * %BSON_VALIDATE_DOT_KEYS: Check that keys do not contain a period.
- * %BSON_VALIDATE_UTF8_ALLOW_NULL: Allow NUL bytes in UTF-8 text.
- * %BSON_VALIDATE_EMPTY_KEYS: Prohibit zero-length field names
+ * Additionally, if validation fails, then the error code set on a `bson_error_t`
+ * will have the value corresponding to the reason that validation failed.
  */
 typedef enum {
+   /**
+    * @brief No special validation behavior specified.
+    */
    BSON_VALIDATE_NONE = 0,
+   /**
+    * @brief Check that all text components of the BSON data are valid UTF-8.
+    *
+    * Note that this will also cause validation to reject valid text that contains
+    * a null character. This can be changed by also passing
+    * `BSON_VALIDATE_UTF8_ALLOW_NULL`
+    */
    BSON_VALIDATE_UTF8 = (1 << 0),
+   /**
+    * @brief Check that element keys do not begin with an ASCII dollar `$`
+    */
    BSON_VALIDATE_DOLLAR_KEYS = (1 << 1),
+   /**
+    * @brief Check that element keys do not contain an ASCII period `.`
+    */
    BSON_VALIDATE_DOT_KEYS = (1 << 2),
+   /**
+    * @brief If set then it is *not* an error for a UTF-8 string to contain
+    * embedded null characters.
+    *
+    * This has no effect unless `BSON_VALIDATE_UTF8` is also passed.
+    */
    BSON_VALIDATE_UTF8_ALLOW_NULL = (1 << 3),
+   /**
+    * @brief Check that no element key is a zero-length empty string.
+    */
    BSON_VALIDATE_EMPTY_KEYS = (1 << 4),
+   /**
+    * @brief This is not a flag that controls behavior, but is instead used to indicate
+    * that a BSON document is corrupted in some way. This is the value that will
+    * appear as an error code.
+    *
+    * Passing this as a flag has no effect.
+    */
+   BSON_VALIDATE_CORRUPT = (1 << 5),
 } bson_validate_flags_t;