diff options
author | Nikita Lapkov <nikita.lapkov@mongodb.com> | 2021-05-14 12:32:17 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2021-06-11 16:05:02 +0000 |
commit | 8e9a4ff28dbf9bd50455aa99b7986e030f9d5923 (patch) | |
tree | 38457bb0ad0e764bd44302e9a0eef40773afa7c1 | |
parent | d984013330025d5b65dfdeabf8c72b0cba997204 (diff) | |
download | mongo-8e9a4ff28dbf9bd50455aa99b7986e030f9d5923.tar.gz |
SERVER-56516 Fix undefined behaviour in $slice arguments parsing
-rw-r--r-- | src/mongo/db/query/projection_ast_test.cpp | 62 | ||||
-rw-r--r-- | src/mongo/db/query/projection_parser.cpp | 13 |
2 files changed, 67 insertions, 8 deletions
diff --git a/src/mongo/db/query/projection_ast_test.cpp b/src/mongo/db/query/projection_ast_test.cpp index b362ef8b3d3..df6238f5a54 100644 --- a/src/mongo/db/query/projection_ast_test.cpp +++ b/src/mongo/db/query/projection_ast_test.cpp @@ -29,6 +29,7 @@ #include "mongo/platform/basic.h" +#include <boost/optional/optional_io.hpp> #include <map> #include <string> #include <vector> @@ -513,6 +514,8 @@ TEST_F(ProjectionASTTest, ParserErrorsOnCollisionIdenticalField) { TEST_F(ProjectionASTTest, ParserErrorsOnSliceWithWrongNumberOfArguments) { ASSERT_THROWS_CODE( parseWithFindFeaturesEnabled(fromjson("{'a': {$slice: []}}")), DBException, 28667); + ASSERT_THROWS_CODE( + parseWithFindFeaturesEnabled(fromjson("{'a': {$slice: [1]}}")), DBException, 28667); ASSERT_THROWS_CODE(parseWithFindFeaturesEnabled(fromjson("{'a': {$slice: [1, 2, 3, 4]}}")), DBException, 28667); @@ -526,6 +529,65 @@ TEST_F(ProjectionASTTest, ParserErrorsOnSliceWithWrongArgumentType) { parseWithFindFeaturesEnabled(fromjson("{'a': {$slice: {foo: 1}}}")), DBException, 28667); } +TEST_F(ProjectionASTTest, ParserErrorsOnFindSliceWithSpecialValuesAsArguments) { + const StringData fieldName = "a"; + + auto checkSliceArguments = [&](const BSONObj& projection, + int expectedLimit, + boost::optional<int> expectedSkip) { + auto parsedProjection = parseWithFindFeaturesEnabled(projection); + auto astNode = parsedProjection.root()->getChild(fieldName); + ASSERT(astNode); + auto sliceAstNode = dynamic_cast<const projection_ast::ProjectionSliceASTNode*>(astNode); + ASSERT(sliceAstNode); + ASSERT_EQ(sliceAstNode->limit(), expectedLimit); + ASSERT_EQ(sliceAstNode->skip(), expectedSkip); + }; + + 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) { + constexpr auto expectedValue = std::numeric_limits<int>::max(); + checkSliceArguments( + BSON(fieldName << BSON("$slice" << element)), expectedValue, boost::none); + checkSliceArguments( + BSON(fieldName << BSON("$slice" << BSON_ARRAY(1 << element))), expectedValue, 1); + checkSliceArguments( + BSON(fieldName << BSON("$slice" << BSON_ARRAY(element << 1))), 1, expectedValue); + checkSliceArguments(BSON(fieldName << BSON("$slice" << BSON_ARRAY(element << element))), + expectedValue, + expectedValue); + } + + 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) { + const auto expectedValue = std::numeric_limits<int>::min(); + checkSliceArguments( + BSON(fieldName << BSON("$slice" << element)), expectedValue, boost::none); + checkSliceArguments( + BSON(fieldName << BSON("$slice" << BSON_ARRAY(element << 1))), 1, expectedValue); + } + + 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) { + constexpr auto expectedValue = 0; + checkSliceArguments( + BSON(fieldName << BSON("$slice" << element)), expectedValue, boost::none); + checkSliceArguments( + BSON(fieldName << BSON("$slice" << BSON_ARRAY(element << 1))), 1, expectedValue); + } +} + TEST_F(ProjectionASTTest, ParserErrorsOnInvalidElemMatchArgument) { ASSERT_THROWS_CODE( parseWithFindFeaturesEnabled(fromjson("{a: {$elemMatch: []}}")), DBException, 31274); diff --git a/src/mongo/db/query/projection_parser.cpp b/src/mongo/db/query/projection_parser.cpp index cae66cf48bf..0df416a2f1b 100644 --- a/src/mongo/db/query/projection_parser.cpp +++ b/src/mongo/db/query/projection_parser.cpp @@ -230,8 +230,8 @@ void attemptToParseFindSlice(ParseContext* parseCtx, if (subObj.firstElement().isNumber()) { addNodeAtPath(parent, path, - std::make_unique<ProjectionSliceASTNode>(boost::none, - subObj.firstElement().numberInt())); + std::make_unique<ProjectionSliceASTNode>( + boost::none, subObj.firstElement().safeNumberInt())); } else if (subObj.firstElementType() == BSONType::Array) { BSONObj arr = subObj.firstElement().embeddedObject(); uassert(31272, "$slice array argument should be of form [skip, limit]", arr.nFields() == 2); @@ -248,13 +248,10 @@ void attemptToParseFindSlice(ParseContext* parseCtx, << limitElt.type(), limitElt.isNumber()); - uassert(31259, - str::stream() << "$slice limit must be positive, got " << limitElt.numberInt(), - limitElt.numberInt() > 0); + auto limit = limitElt.safeNumberInt(); + uassert(31259, str::stream() << "$slice limit must be positive, got " << limit, limit > 0); addNodeAtPath( - parent, - path, - std::make_unique<ProjectionSliceASTNode>(skipElt.numberInt(), limitElt.numberInt())); + parent, path, std::make_unique<ProjectionSliceASTNode>(skipElt.safeNumberInt(), limit)); } else { uasserted(31273, "$slice only supports numbers and [skip, limit] arrays"); } |