SERVER-30875 add requireOwnedObjects() to scoper3.4.8-rc1r3.4.8

Add a flag to JS scopes that requires that bson objects bound to the
scope be owned.  This should allow for more easy auditing of scopes that
don't explicitly manage lifetime with advanceGeneration.

(cherry picked from commit 79b47945a6aae707d44e05669d991d86b157a14b)

9 files changed, 79 insertions, 3 deletions

diff --git a/src/mongo/db/commands/mr.cpp b/src/mongo/db/commands/mr.cpp
index 4aeed6d5978..7b7d16d17d3 100644
--- a/src/mongo/db/commands/mr.cpp
+++ b/src/mongo/db/commands/mr.cpp
@@ -836,6 +836,7 @@ void State::init() {
         AuthorizationSession::get(Client::getCurrent())->getAuthenticatedUserNamesToken();
     _scope.reset(getGlobalScriptEngine()->newScopeForCurrentThread());
     _scope->registerOperation(_txn);
+    _scope->requireOwnedObjects();
     _scope->setLocalDB(_config.dbname);
     _scope->loadStored(_txn, true);
 
@@ -1522,6 +1523,7 @@ public:
                 BSONObj o;
                 PlanExecutor::ExecState execState;
                 while (PlanExecutor::ADVANCED == (execState = exec->getNext(&o, NULL))) {
+                    o = o.getOwned();  // we will be accessing outside of the lock
                     // check to see if this is a new object we don't own yet
                     // because of a chunk migration
                     if (collMetadata) {
diff --git a/src/mongo/dbtests/jstests.cpp b/src/mongo/dbtests/jstests.cpp
index 13e71a0a74a..c0421910c2f 100644
--- a/src/mongo/dbtests/jstests.cpp
+++ b/src/mongo/dbtests/jstests.cpp
@@ -2388,6 +2388,49 @@ public:
     }
 };
 
+class RequiresOwnedObjects {
+public:
+    void run() {
+        char buf[] = {5, 0, 0, 0, 0};
+        BSONObj unowned(buf);
+        BSONObj owned = unowned.getOwned();
+
+        ASSERT(!unowned.isOwned());
+        ASSERT(owned.isOwned());
+
+        // Ensure that by default we can bind owned and unowned
+        {
+            unique_ptr<Scope> s(getGlobalScriptEngine()->newScope());
+            s->setObject("unowned", unowned, true);
+            s->setObject("owned", owned, true);
+        }
+
+        // After we set the flag, we should only be able to set owned
+        {
+            unique_ptr<Scope> s(getGlobalScriptEngine()->newScope());
+            s->requireOwnedObjects();
+            s->setObject("owned", owned, true);
+
+            bool threwException = false;
+            try {
+                s->setObject("unowned", unowned, true);
+            } catch (...) {
+                threwException = true;
+
+                auto status = exceptionToStatus();
+
+                ASSERT_EQUALS(status.code(), ErrorCodes::BadValue);
+            }
+
+            ASSERT(threwException);
+
+            // after resetting, we can set unowned's again
+            s->reset();
+            s->setObject("unowned", unowned, true);
+        }
+    }
+};
+
 class All : public Suite {
 public:
     All() : Suite("js") {}
@@ -2444,6 +2487,7 @@ public:
 
         add<RecursiveInvoke>();
         add<ErrorCodeFromInvoke>();
+        add<RequiresOwnedObjects>();
 
         add<RoundTripTests::DBRefTest>();
         add<RoundTripTests::DBPointerTest>();
diff --git a/src/mongo/scripting/engine.cpp b/src/mongo/scripting/engine.cpp
index 3e9a5c542cf..684d4d747c9 100644
--- a/src/mongo/scripting/engine.cpp
+++ b/src/mongo/scripting/engine.cpp
@@ -427,6 +427,9 @@ public:
     void advanceGeneration() {
         _real->advanceGeneration();
     }
+    void requireOwnedObjects() override {
+        _real->requireOwnedObjects();
+    }
     bool isKillPending() const {
         return _real->isKillPending();
     }
diff --git a/src/mongo/scripting/engine.h b/src/mongo/scripting/engine.h
index 2b2ab0ff14d..3fcfeff1f8b 100644
--- a/src/mongo/scripting/engine.h
+++ b/src/mongo/scripting/engine.h
@@ -103,6 +103,8 @@ public:
 
     virtual void advanceGeneration() = 0;
 
+    virtual void requireOwnedObjects() = 0;
+
     virtual ScriptingFunction createFunction(const char* code);
 
     /**
diff --git a/src/mongo/scripting/mozjs/bson.cpp b/src/mongo/scripting/mozjs/bson.cpp
index 5a2ebd0dfed..a2881c44664 100644
--- a/src/mongo/scripting/mozjs/bson.cpp
+++ b/src/mongo/scripting/mozjs/bson.cpp
@@ -59,13 +59,17 @@ namespace {
  * the appearance of mutable state on the read/write versions.
  */
 struct BSONHolder {
-    BSONHolder(const BSONObj& obj, const BSONObj* parent, std::size_t generation, bool ro)
+    BSONHolder(const BSONObj& obj, const BSONObj* parent, const MozJSImplScope* scope, bool ro)
         : _obj(obj),
-          _generation(generation),
+          _generation(scope->getGeneration()),
           _isOwned(obj.isOwned() || (parent && parent->isOwned())),
           _resolved(false),
           _readOnly(ro),
           _altered(false) {
+        uassert(
+            ErrorCodes::BadValue,
+            "Attempt to bind an unowned BSON Object to a JS scope marked as requiring ownership",
+            _isOwned || (!scope->requiresOwnedObjects()));
         if (parent) {
             _parent.emplace(*parent);
         }
@@ -107,7 +111,7 @@ void BSONInfo::make(
     auto scope = getScope(cx);
 
     scope->getProto<BSONInfo>().newObject(obj);
-    JS_SetPrivate(obj, scope->trackedNew<BSONHolder>(bson, parent, scope->getGeneration(), ro));
+    JS_SetPrivate(obj, scope->trackedNew<BSONHolder>(bson, parent, scope, ro));
 }
 
 void BSONInfo::finalize(JSFreeOp* fop, JSObject* obj) {
diff --git a/src/mongo/scripting/mozjs/implscope.cpp b/src/mongo/scripting/mozjs/implscope.cpp
index c37ebc0cb2a..1a9fcc3fddc 100644
--- a/src/mongo/scripting/mozjs/implscope.cpp
+++ b/src/mongo/scripting/mozjs/implscope.cpp
@@ -403,6 +403,7 @@ MozJSImplScope::MozJSImplScope(MozJSScriptEngine* engine)
       _connectState(ConnectState::Not),
       _status(Status::OK()),
       _generation(0),
+      _requireOwnedObjects(false),
       _hasOutOfMemoryException(false),
       _binDataProto(_context),
       _bsonProto(_context),
@@ -838,6 +839,7 @@ void MozJSImplScope::reset() {
     unregisterOperation();
     _pendingKill.store(false);
     _pendingGC.store(false);
+    _requireOwnedObjects = false;
     advanceGeneration();
 }
 
@@ -951,6 +953,14 @@ void MozJSImplScope::advanceGeneration() {
     _generation++;
 }
 
+void MozJSImplScope::requireOwnedObjects() {
+    _requireOwnedObjects = true;
+}
+
+bool MozJSImplScope::requiresOwnedObjects() const {
+    return _requireOwnedObjects;
+}
+
 const std::string& MozJSImplScope::getParentStack() const {
     return _parentStack;
 }
diff --git a/src/mongo/scripting/mozjs/implscope.h b/src/mongo/scripting/mozjs/implscope.h
index df37553e181..ea9b35d2cf4 100644
--- a/src/mongo/scripting/mozjs/implscope.h
+++ b/src/mongo/scripting/mozjs/implscope.h
@@ -309,6 +309,10 @@ public:
 
     void advanceGeneration() override;
 
+    void requireOwnedObjects() override;
+
+    bool requiresOwnedObjects() const;
+
     JS::HandleId getInternedStringId(InternedString name) {
         return _internedStrings.getInternedString(name);
     }
@@ -402,6 +406,7 @@ private:
     Status _status;
     std::string _parentStack;
     std::size_t _generation;
+    bool _requireOwnedObjects;
     bool _hasOutOfMemoryException;
 
     WrapType<BinDataInfo> _binDataProto;
diff --git a/src/mongo/scripting/mozjs/proxyscope.cpp b/src/mongo/scripting/mozjs/proxyscope.cpp
index 6a6d20f4448..05697468f07 100644
--- a/src/mongo/scripting/mozjs/proxyscope.cpp
+++ b/src/mongo/scripting/mozjs/proxyscope.cpp
@@ -122,6 +122,10 @@ void MozJSProxyScope::advanceGeneration() {
     run([&] { _implScope->advanceGeneration(); });
 }
 
+void MozJSProxyScope::requireOwnedObjects() {
+    run([&] { _implScope->requireOwnedObjects(); });
+}
+
 double MozJSProxyScope::getNumber(const char* field) {
     double out;
     run([&] { out = _implScope->getNumber(field); });
diff --git a/src/mongo/scripting/mozjs/proxyscope.h b/src/mongo/scripting/mozjs/proxyscope.h
index 451981330a1..4dd69a3ebe9 100644
--- a/src/mongo/scripting/mozjs/proxyscope.h
+++ b/src/mongo/scripting/mozjs/proxyscope.h
@@ -129,6 +129,8 @@ public:
 
     void advanceGeneration() override;
 
+    void requireOwnedObjects() override;
+
     double getNumber(const char* field) override;
     int getNumberInt(const char* field) override;
     long long getNumberLongLong(const char* field) override;