SERVER-51720 Fix BSONElement::_binDataVector bounds for ByteArrayDeprecated

2 files changed, 48 insertions, 10 deletions

diff --git a/src/mongo/bson/bsonelement.h b/src/mongo/bson/bsonelement.h
index c868654d65e..6ec4137945f 100644
--- a/src/mongo/bson/bsonelement.h
+++ b/src/mongo/bson/bsonelement.h
@@ -556,16 +556,11 @@ public:
     }
 
     std::vector<uint8_t> _binDataVector() const {
-        if (binDataType() != ByteArrayDeprecated) {
-            return std::vector<uint8_t>(reinterpret_cast<const uint8_t*>(value()) + 5,
-                                        reinterpret_cast<const uint8_t*>(value()) + 5 +
-                                            valuestrsize());
-        } else {
-            // Skip the extra int32 size
-            return std::vector<uint8_t>(reinterpret_cast<const uint8_t*>(value()) + 4,
-                                        reinterpret_cast<const uint8_t*>(value()) + 4 +
-                                            valuestrsize() - 4);
-        }
+        auto first = reinterpret_cast<const uint8_t*>(value()) + 5;
+        auto last = first + valuestrsize();
+        if (binDataType() == ByteArrayDeprecated)
+            first += std::min<size_t>(4, last - first);  // skip extra int32 size.
+        return {first, last};
     }
 
     /** Retrieve the regex std::string for a Regex element */
diff --git a/src/mongo/bson/bsonelement_test.cpp b/src/mongo/bson/bsonelement_test.cpp
index 9fd3ac54e0a..6bcd5abd820 100644
--- a/src/mongo/bson/bsonelement_test.cpp
+++ b/src/mongo/bson/bsonelement_test.cpp
@@ -29,6 +29,9 @@
 
 #include "mongo/platform/basic.h"
 
+#include <array>
+#include <fmt/format.h>
+
 #include "mongo/bson/bsonelement.h"
 #include "mongo/bson/bsonobj.h"
 #include "mongo/bson/bsonobjbuilder.h"
@@ -38,6 +41,8 @@
 namespace mongo {
 namespace {
 
+using namespace fmt::literals;
+
 TEST(BSONElement, BinDataToString) {
     BSONObjBuilder builder;
     unsigned char bintype0[] = {0xDE, 0xEA, 0xBE, 0xEF, 0x01};  // Random BinData shorter than UUID
@@ -85,6 +90,44 @@ TEST(BSONElement, BinDataToString) {
               "62696E6172792064617461007769746820616E20756E6B6E6F776E2074797065)");
 }
 
+std::string vecStr(std::vector<uint8_t> v) {
+    std::string r = "[";
+    StringData sep;
+    for (const uint8_t& b : v) {
+        r += "{}{:02x}"_format(sep, (unsigned)b);
+        sep = ","_sd;
+    }
+    r += "]";
+    return r;
+}
+
+TEST(BSONElement, BinDataVectorWithByteArrayDeprecated) {
+    // ByteArrayDeprecated has a nonsense 4-byte redundant length field
+    // that _binDataVector should ignore.
+    std::vector<uint8_t> payload{'1', '2', '3', '4', '5', '6', '7', '8', '9'};
+    std::vector<uint8_t> input = payload;
+
+    // Insert a 4-byte prefix for the ignored ByteArrayDeprecated "length"
+    std::array<uint8_t, 4> ignoredPrefix{0xcc, 0xdd, 0xee, 0xff};
+    input.insert(input.begin(), ignoredPrefix.begin(), ignoredPrefix.end());
+
+    ASSERT_EQ(vecStr(BSONObjBuilder{}
+                         .appendBinData("f", input.size(), ByteArrayDeprecated, input.data())
+                         .obj()["f"]
+                         ._binDataVector()),
+              vecStr(payload));
+}
+
+TEST(BSONElement, BinDataVectorWithBinDataGeneral) {
+    std::vector<uint8_t> payload{'1', '2', '3', '4', '5', '6', '7', '8', '9'};
+    std::vector<uint8_t> input = payload;
+    ASSERT_EQ(vecStr(BSONObjBuilder{}
+                         .appendBinData("f", input.size(), BinDataGeneral, input.data())
+                         .obj()["f"]
+                         ._binDataVector()),
+              vecStr(payload));
+}
+
 
 TEST(BSONElement, TimestampToString) {