SERVER-56516 Fix undefined behaviour in $slice arguments parsing

5 files changed, 100 insertions, 15 deletions

diff --git a/src/mongo/bson/bsonelement.h b/src/mongo/bson/bsonelement.h
index 151872be545..86bd8ed9408 100644
--- a/src/mongo/bson/bsonelement.h
+++ b/src/mongo/bson/bsonelement.h
@@ -341,6 +341,14 @@ public:
 
     /** Retrieve int value for the element safely.  Zero returned if not a number. */
     int numberInt() const;
+
+    /** Like numberInt() but with well-defined behavior for doubles that
+     *  are NaNs, or too large/small to be represented as int.
+     *  NaNs -> 0
+     *  very large doubles -> INT_MAX
+     *  very small doubles -> INT_MIN  */
+    int safeNumberInt() const;
+
     /** Retrieve long value for the element safely.  Zero returned if not a number.
      *  Behavior is not defined for double values that are NaNs, or too large/small
      *  to be represented by long longs */
@@ -863,6 +871,16 @@ inline int BSONElement::numberInt() const {
     }
 }
 
+/** Like numberInt() but with well-defined behavior for doubles that
+ *  are NaNs, or too large/small to be represented as int.
+ *  NaNs -> 0
+ *  very large doubles -> INT_MAX
+ *  very small doubles -> INT_MIN  */
+inline int BSONElement::safeNumberInt() const {
+    return static_cast<int>(std::clamp<long long>(
+        safeNumberLong(), std::numeric_limits<int>::min(), std::numeric_limits<int>::max()));
+}
+
 /** Retrieve long value for the element safely.  Zero returned if not a number. */
 inline long long BSONElement::numberLong() const {
     switch (type()) {
diff --git a/src/mongo/db/exec/projection_exec.cpp b/src/mongo/db/exec/projection_exec.cpp
index bc2a4a42d65..bacd618fd3e 100644
--- a/src/mongo/db/exec/projection_exec.cpp
+++ b/src/mongo/db/exec/projection_exec.cpp
@@ -63,7 +63,7 @@ ProjectionExec::ProjectionExec(OperationContext* opCtx,
             BSONElement e2 = obj.firstElement();
             if (e2.fieldNameStringData() == "$slice") {
                 if (e2.isNumber()) {
-                    int i = e2.numberInt();
+                    long long i = e2.safeNumberInt();
                     if (i < 0) {
                         add(e.fieldName(), i, -i);  // limit is now positive
                     } else {
@@ -75,8 +75,8 @@ ProjectionExec::ProjectionExec(OperationContext* opCtx,
                     invariant(2 == arr.nFields());
 
                     BSONObjIterator it(arr);
-                    int skip = it.next().numberInt();
-                    int limit = it.next().numberInt();
+                    int skip = it.next().safeNumberInt();
+                    int limit = it.next().safeNumberInt();
 
                     invariant(limit > 0);
 
@@ -174,7 +174,7 @@ void ProjectionExec::add(const string& field, bool include) {
     }
 }
 
-void ProjectionExec::add(const string& field, int skip, int limit) {
+void ProjectionExec::add(const string& field, long long skip, long long limit) {
     _special = true;  // can't include or exclude whole object
 
     if (field.empty()) {  // this is the field the user referred to
@@ -399,11 +399,11 @@ Status ProjectionExec::projectHelper(const BSONObj& in,
 }
 
 void ProjectionExec::appendArray(BSONObjBuilder* bob, const BSONObj& array, bool nested) const {
-    int skip = nested ? 0 : _skip;
-    int limit = nested ? -1 : _limit;
+    long long skip = nested ? 0 : _skip;
+    long long limit = nested ? -1 : _limit;
 
     if (skip < 0) {
-        skip = std::max(0, skip + array.nFields());
+        skip = std::max(0ll, skip + array.nFields());
     }
 
     int index = 0;
diff --git a/src/mongo/db/exec/projection_exec.h b/src/mongo/db/exec/projection_exec.h
index 805be89ef44..1b955ac77bd 100644
--- a/src/mongo/db/exec/projection_exec.h
+++ b/src/mongo/db/exec/projection_exec.h
@@ -186,7 +186,7 @@ private:
     /**
      * Add 'field' as a field name that is sliced as part of the projection.
      */
-    void add(const std::string& field, int skip, int limit);
+    void add(const std::string& field, long long skip, long long limit);
 
     //
     // Execution
@@ -241,8 +241,8 @@ private:
     bool _includeID = true;
 
     // Arguments from the $slice operator.
-    int _skip = 0;
-    int _limit = -1;
+    long long _skip = 0;
+    long long _limit = -1;
 
     // Used for $elemMatch and positional operator ($)
     Matchers _matchers;
diff --git a/src/mongo/db/exec/projection_exec_test.cpp b/src/mongo/db/exec/projection_exec_test.cpp
index 9c04e20c2d0..033ac31c590 100644
--- a/src/mongo/db/exec/projection_exec_test.cpp
+++ b/src/mongo/db/exec/projection_exec_test.cpp
@@ -65,15 +65,13 @@ std::unique_ptr<MatchExpression> parseMatchExpression(const BSONObj& obj) {
  * ProjectionExec::projectCovered().
  */
 boost::optional<std::string> project(
-    const char* specStr,
-    const char* queryStr,
+    const BSONObj& spec,
+    const BSONObj& query,
     const stdx::variant<const char*, const IndexKeyDatum> objStrOrDatum,
     const boost::optional<const CollatorInterface&> collator = boost::none,
     const BSONObj& sortKey = BSONObj(),
     const double textScore = 0.0) {
     // Create projection exec object.
-    BSONObj spec = fromjson(specStr);
-    BSONObj query = fromjson(queryStr);
     std::unique_ptr<MatchExpression> queryExpression = parseMatchExpression(query);
     QueryTestServiceContext serviceCtx;
     auto opCtx = serviceCtx.makeOperationContext();
@@ -94,6 +92,18 @@ boost::optional<std::string> project(
         return boost::make_optional(projected.getValue().toString());
 }
 
+boost::optional<std::string> project(
+    const char* specStr,
+    const char* queryStr,
+    const stdx::variant<const char*, const IndexKeyDatum> objStrOrDatum,
+    const boost::optional<const CollatorInterface&> collator = boost::none,
+    const BSONObj& sortKey = BSONObj(),
+    const double textScore = 0.0) {
+    BSONObj spec = fromjson(specStr);
+    BSONObj query = fromjson(queryStr);
+    return project(spec, query, objStrOrDatum, collator, sortKey, textScore);
+}
+
 //
 // position $
 //
@@ -184,6 +194,63 @@ TEST(ProjectionExecTest, TransformSliceSkipLimit) {
               project("{a: {$slice: [10, 10]}}", "{}", "{a: [4, 6, 8]}"));
 }
 
+TEST(ProjectionExecTest, SliceWithSpecialValuesAsArguments) {
+    auto assertSliceResult =
+        [&](const BSONObj& spec, const char* inputDocument, const char* expectedResult) {
+            ASSERT_EQ(boost::make_optional(std::string(expectedResult)),
+                      project(spec, BSONObj(), inputDocument));
+        };
+
+    const auto positiveClamping =
+        BSON_ARRAY((static_cast<long long>(std::numeric_limits<int>::max()) + 1)
+                   << std::numeric_limits<long long>::max() << std::numeric_limits<double>::max()
+                   << std::numeric_limits<double>::infinity() << Decimal128::kPositiveInfinity
+                   << Decimal128::kLargestPositive);
+    for (const auto& element : positiveClamping) {
+        assertSliceResult(
+            BSON("a" << BSON("$slice" << element)), "{ a: [ 1, 2, 3 ] }", "{ a: [ 1, 2, 3 ] }");
+
+        assertSliceResult(BSON("a" << BSON("$slice" << BSON_ARRAY(1 << element))),
+                          "{ a: [1, 2, 3] }",
+                          "{ a: [ 2, 3 ] }");
+
+        assertSliceResult(BSON("a" << BSON("$slice" << BSON_ARRAY(element << 1))),
+                          "{ a: [ 1, 2, 3 ] }",
+                          "{ a: [] }");
+
+        assertSliceResult(BSON("a" << BSON("$slice" << BSON_ARRAY(element << element))),
+                          "{ a: [ 1, 2, 3 ] }",
+                          "{ a: [] }");
+    }
+
+    const auto negativeClamping =
+        BSON_ARRAY((static_cast<long long>(std::numeric_limits<int>::min()) - 1)
+                   << std::numeric_limits<long long>::min() << std::numeric_limits<double>::lowest()
+                   << -std::numeric_limits<double>::infinity() << Decimal128::kNegativeInfinity
+                   << Decimal128::kLargestNegative);
+    for (const auto& element : negativeClamping) {
+        assertSliceResult(
+            BSON("a" << BSON("$slice" << element)), "{ a: [ 1, 2, 3 ] }", "{ a: [ 1, 2, 3 ] }");
+
+        assertSliceResult(BSON("a" << BSON("$slice" << BSON_ARRAY(element << 1))),
+                          "{ a: [ 1, 2, 3 ] }",
+                          "{ a: [ 1 ] }");
+    }
+
+    const auto convertionToZero =
+        BSON_ARRAY(0ll << 0.0 << 0.3 << -0.3 << std::numeric_limits<double>::quiet_NaN()
+                       << Decimal128::kNegativeNaN << Decimal128::kPositiveNaN
+                       << Decimal128::kSmallestPositive << Decimal128::kSmallestNegative);
+    for (const auto& element : convertionToZero) {
+        assertSliceResult(
+            BSON("a" << BSON("$slice" << element)), "{ a: [ 1, 2, 3 ] }", "{ a: [] }");
+
+        assertSliceResult(BSON("a" << BSON("$slice" << BSON_ARRAY(element << 1))),
+                          "{ a: [ 1, 2, 3 ] }",
+                          "{ a: [ 1 ] }");
+    }
+}
+
 //
 // Dotted projections.
 //
diff --git a/src/mongo/db/query/parsed_projection.cpp b/src/mongo/db/query/parsed_projection.cpp
index 359ad5c23d8..97f5f58224c 100644
--- a/src/mongo/db/query/parsed_projection.cpp
+++ b/src/mongo/db/query/parsed_projection.cpp
@@ -90,7 +90,7 @@ Status ParsedProjection::make(OperationContext* opCtx,
                     BSONObjIterator it(arr);
                     // Skip over 'skip'.
                     it.next();
-                    int limit = it.next().numberInt();
+                    int limit = it.next().safeNumberInt();
                     if (limit <= 0) {
                         return Status(ErrorCodes::BadValue, "$slice limit must be positive");
                     }