SERVER-31664 Fix use after free of OperationContext by ExpressionDate*

Addresses the case where a time zone expression lives within a collection
validator as part of a $expr expression. In this case, the Expression
will outlive the OperationContext it was created under.

9 files changed, 46 insertions, 22 deletions

diff --git a/src/mongo/db/catalog/collection_impl.cpp b/src/mongo/db/catalog/collection_impl.cpp
index 297a9732515..689b340923a 100644
--- a/src/mongo/db/catalog/collection_impl.cpp
+++ b/src/mongo/db/catalog/collection_impl.cpp
@@ -289,8 +289,13 @@ StatusWithMatchExpression CollectionImpl::parseValidator(
     }
 
     boost::intrusive_ptr<ExpressionContext> expCtx(new ExpressionContext(opCtx, _collator.get()));
-    auto statusWithMatcher = MatchExpressionParser::parse(
-        validator, std::move(expCtx), ExtensionsCallbackNoop(), allowedFeatures);
+
+    // The MatchExpression and contained ExpressionContext created as part of the validator are
+    // owned by the Collection and will outlive the OperationContext they were created under.
+    expCtx->opCtx = nullptr;
+
+    auto statusWithMatcher =
+        MatchExpressionParser::parse(validator, expCtx, ExtensionsCallbackNoop(), allowedFeatures);
     if (!statusWithMatcher.isOK())
         return statusWithMatcher.getStatus();
 
diff --git a/src/mongo/db/operation_context.h b/src/mongo/db/operation_context.h
index 5f1cbec5954..4d5f05ceb85 100644
--- a/src/mongo/db/operation_context.h
+++ b/src/mongo/db/operation_context.h
@@ -262,9 +262,14 @@ public:
         stdx::condition_variable& cv, stdx::unique_lock<stdx::mutex>& m, Date_t deadline) noexcept;
 
     /**
-     * Returns the service context under which this operation context runs.
+     * Returns the service context under which this operation context runs, or nullptr if there is
+     * no such service context.
      */
     ServiceContext* getServiceContext() const {
+        if (!_client) {
+            return nullptr;
+        }
+
         return _client->getServiceContext();
     }
 
diff --git a/src/mongo/db/pipeline/expression.cpp b/src/mongo/db/pipeline/expression.cpp
index e849b778378..2ceb6302392 100644
--- a/src/mongo/db/pipeline/expression.cpp
+++ b/src/mongo/db/pipeline/expression.cpp
@@ -969,6 +969,8 @@ namespace {
 boost::optional<TimeZone> makeTimeZone(const TimeZoneDatabase* tzdb,
                                        const Document& root,
                                        intrusive_ptr<Expression> _timeZone) {
+    invariant(tzdb);
+
     if (!_timeZone) {
         return mongo::TimeZoneDatabase::utcZone();
     }
@@ -1230,8 +1232,7 @@ Value ExpressionDateFromParts::evaluate(const Document& root) const {
         return Value(BSONNULL);
     }
 
-    auto timeZone = makeTimeZone(
-        TimeZoneDatabase::get(getExpressionContext()->opCtx->getServiceContext()), root, _timeZone);
+    auto timeZone = makeTimeZone(getExpressionContext()->timeZoneDatabase, root, _timeZone);
 
     if (!timeZone) {
         return Value(BSONNULL);
@@ -1372,8 +1373,7 @@ Value ExpressionDateFromString::serialize(bool explain) const {
 Value ExpressionDateFromString::evaluate(const Document& root) const {
     const Value dateString = _dateString->evaluate(root);
 
-    auto timeZone = makeTimeZone(
-        TimeZoneDatabase::get(getExpressionContext()->opCtx->getServiceContext()), root, _timeZone);
+    auto timeZone = makeTimeZone(getExpressionContext()->timeZoneDatabase, root, _timeZone);
 
     if (!timeZone || dateString.nullish()) {
         return Value(BSONNULL);
@@ -1387,9 +1387,7 @@ Value ExpressionDateFromString::evaluate(const Document& root) const {
             dateString.getType() == BSONType::String);
     const std::string& dateTimeString = dateString.getString();
 
-    auto tzdb = TimeZoneDatabase::get(getExpressionContext()->opCtx->getServiceContext());
-
-    return Value(tzdb->fromString(dateTimeString, timeZone));
+    return Value(getExpressionContext()->timeZoneDatabase->fromString(dateTimeString, timeZone));
 }
 
 void ExpressionDateFromString::_doAddDependencies(DepsTracker* deps) const {
@@ -1494,8 +1492,7 @@ boost::optional<int> ExpressionDateToParts::evaluateIso8601Flag(const Document&
 Value ExpressionDateToParts::evaluate(const Document& root) const {
     const Value date = _date->evaluate(root);
 
-    auto timeZone = makeTimeZone(
-        TimeZoneDatabase::get(getExpressionContext()->opCtx->getServiceContext()), root, _timeZone);
+    auto timeZone = makeTimeZone(getExpressionContext()->timeZoneDatabase, root, _timeZone);
     if (!timeZone) {
         return Value(BSONNULL);
     }
@@ -1622,8 +1619,7 @@ Value ExpressionDateToString::serialize(bool explain) const {
 Value ExpressionDateToString::evaluate(const Document& root) const {
     const Value date = _date->evaluate(root);
 
-    auto timeZone = makeTimeZone(
-        TimeZoneDatabase::get(getExpressionContext()->opCtx->getServiceContext()), root, _timeZone);
+    auto timeZone = makeTimeZone(getExpressionContext()->timeZoneDatabase, root, _timeZone);
     if (!timeZone) {
         return Value(BSONNULL);
     }
diff --git a/src/mongo/db/pipeline/expression.h b/src/mongo/db/pipeline/expression.h
index 10547772cf8..39396202dc8 100644
--- a/src/mongo/db/pipeline/expression.h
+++ b/src/mongo/db/pipeline/expression.h
@@ -519,8 +519,10 @@ public:
                               << timeZoneId.toString()
                               << ")",
                 timeZoneId.getType() == BSONType::String);
-        auto timeZone = TimeZoneDatabase::get(getExpressionContext()->opCtx->getServiceContext())
-                            ->getTimeZone(timeZoneId.getString());
+
+        invariant(getExpressionContext()->timeZoneDatabase);
+        auto timeZone =
+            getExpressionContext()->timeZoneDatabase->getTimeZone(timeZoneId.getString());
         return evaluateDate(date, timeZone);
     }
 
diff --git a/src/mongo/db/pipeline/expression_context.cpp b/src/mongo/db/pipeline/expression_context.cpp
index 71d7c9c9c51..7901b89da30 100644
--- a/src/mongo/db/pipeline/expression_context.cpp
+++ b/src/mongo/db/pipeline/expression_context.cpp
@@ -57,6 +57,9 @@ ExpressionContext::ExpressionContext(OperationContext* opCtx,
 }
 ExpressionContext::ExpressionContext(OperationContext* opCtx, const CollatorInterface* collator)
     : opCtx(opCtx),
+      timeZoneDatabase(opCtx && opCtx->getServiceContext()
+                           ? TimeZoneDatabase::get(opCtx->getServiceContext())
+                           : nullptr),
       variablesParseState(variables.useIdGenerator()),
       _collator(collator),
       _documentComparator(_collator),
@@ -87,7 +90,8 @@ void ExpressionContext::setCollator(const CollatorInterface* collator) {
 
 intrusive_ptr<ExpressionContext> ExpressionContext::copyWith(NamespaceString ns,
                                                              boost::optional<UUID> uuid) const {
-    intrusive_ptr<ExpressionContext> expCtx = new ExpressionContext(std::move(ns));
+    intrusive_ptr<ExpressionContext> expCtx =
+        new ExpressionContext(std::move(ns), timeZoneDatabase);
 
     expCtx->uuid = std::move(uuid);
     expCtx->explain = explain;
diff --git a/src/mongo/db/pipeline/expression_context.h b/src/mongo/db/pipeline/expression_context.h
index 5bce214c776..a5bebd95745 100644
--- a/src/mongo/db/pipeline/expression_context.h
+++ b/src/mongo/db/pipeline/expression_context.h
@@ -42,6 +42,7 @@
 #include "mongo/db/pipeline/value_comparator.h"
 #include "mongo/db/pipeline/variables.h"
 #include "mongo/db/query/collation/collator_interface.h"
+#include "mongo/db/query/datetime/date_time_support.h"
 #include "mongo/db/query/explain_options.h"
 #include "mongo/db/query/tailable_mode.h"
 #include "mongo/util/intrusive_counter.h"
@@ -141,6 +142,8 @@ public:
 
     OperationContext* opCtx;
 
+    const TimeZoneDatabase* timeZoneDatabase;
+
     // Collation requested by the user for this pipeline. Empty if the user did not request a
     // collation.
     BSONObj collation;
@@ -153,8 +156,10 @@ public:
 protected:
     static const int kInterruptCheckPeriod = 128;
 
-    ExpressionContext(NamespaceString nss)
-        : ns(std::move(nss)), variablesParseState(variables.useIdGenerator()) {}
+    ExpressionContext(NamespaceString nss, const TimeZoneDatabase* tzDb)
+        : ns(std::move(nss)),
+          timeZoneDatabase(tzDb),
+          variablesParseState(variables.useIdGenerator()) {}
 
     /**
      * Sets '_ownedCollator' and resets '_collator', 'documentComparator' and 'valueComparator'.
diff --git a/src/mongo/db/pipeline/expression_context_for_test.h b/src/mongo/db/pipeline/expression_context_for_test.h
index b54bb82094f..2747862f52b 100644
--- a/src/mongo/db/pipeline/expression_context_for_test.h
+++ b/src/mongo/db/pipeline/expression_context_for_test.h
@@ -42,13 +42,20 @@ namespace mongo {
  */
 class ExpressionContextForTest : public ExpressionContext {
 public:
+    static constexpr TimeZoneDatabase* kNullTimeZoneDatabase = nullptr;
+
     ExpressionContextForTest()
         : ExpressionContextForTest(NamespaceString{"test"_sd, "namespace"_sd}) {}
 
     ExpressionContextForTest(NamespaceString nss)
-        : ExpressionContext(std::move(nss)), _testOpCtx(_serviceContext.makeOperationContext()) {
+        : ExpressionContext(std::move(nss), kNullTimeZoneDatabase),
+          _testOpCtx(_serviceContext.makeOperationContext()) {
         TimeZoneDatabase::set(_serviceContext.getServiceContext(),
                               stdx::make_unique<TimeZoneDatabase>());
+
+        // As we don't have the TimeZoneDatabase prior to ExpressionContext construction, we must
+        // initialize with a nullptr and set post-construction.
+        timeZoneDatabase = TimeZoneDatabase::get(_serviceContext.getServiceContext());
         opCtx = _testOpCtx.get();
     }
 
diff --git a/src/mongo/db/query/datetime/date_time_support.cpp b/src/mongo/db/query/datetime/date_time_support.cpp
index fc24dd22206..2b6cb8ec078 100644
--- a/src/mongo/db/query/datetime/date_time_support.cpp
+++ b/src/mongo/db/query/datetime/date_time_support.cpp
@@ -68,7 +68,6 @@ long long seconds(Date_t date) {
 }  // namespace
 
 const TimeZoneDatabase* TimeZoneDatabase::get(ServiceContext* serviceContext) {
-    invariant(getTimeZoneDatabase(serviceContext));
     return getTimeZoneDatabase(serviceContext).get();
 }
 
diff --git a/src/mongo/db/query/datetime/date_time_support.h b/src/mongo/db/query/datetime/date_time_support.h
index 9973a198939..9d5b3188abc 100644
--- a/src/mongo/db/query/datetime/date_time_support.h
+++ b/src/mongo/db/query/datetime/date_time_support.h
@@ -346,7 +346,8 @@ public:
     };
 
     /**
-     * Returns the TimeZoneDatabase object associated with the specified service context.
+     * Returns the TimeZoneDatabase object associated with the specified service context or nullptr
+     * if none exists.
      */
     static const TimeZoneDatabase* get(ServiceContext* serviceContext);