SERVER-30875 add requireOwnedObjects() to scope

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 f93dee2b554..a85355cdceb 100644
--- a/src/mongo/db/commands/mr.cpp
+++ b/src/mongo/db/commands/mr.cpp
@@ -787,6 +787,7 @@ void State::init() {
         AuthorizationSession::get(ClientBasic::getCurrent())->getAuthenticatedUserNamesToken();
     _scope.reset(globalScriptEngine->newScopeForCurrentThread());
     _scope->registerOperation(_txn);
+    _scope->requireOwnedObjects();
     _scope->setLocalDB(_config.dbname);
     _scope->loadStored(_txn, true);
 
@@ -1435,6 +1436,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 53cead63a3a..04a9180dbbd 100644
--- a/src/mongo/dbtests/jstests.cpp
+++ b/src/mongo/dbtests/jstests.cpp
@@ -2361,6 +2361,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(globalScriptEngine->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(globalScriptEngine->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") {
@@ -2422,6 +2465,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 7e6628344ab..71c42f2fe74 100644
--- a/src/mongo/scripting/engine.cpp
+++ b/src/mongo/scripting/engine.cpp
@@ -422,6 +422,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 d31311f0114..2fbae5c347e 100644
--- a/src/mongo/scripting/engine.h
+++ b/src/mongo/scripting/engine.h
@@ -105,6 +105,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 1e7e4b7c190..3783bc0755a 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, new BSONHolder(bson, parent, scope->getGeneration(), ro));
+    JS_SetPrivate(obj, new 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 3b260c1558c..e85efd97505 100644
--- a/src/mongo/scripting/mozjs/implscope.cpp
+++ b/src/mongo/scripting/mozjs/implscope.cpp
@@ -322,6 +322,7 @@ MozJSImplScope::MozJSImplScope(MozJSScriptEngine* engine)
       _status(Status::OK()),
       _quickExit(false),
       _generation(0),
+      _requireOwnedObjects(false),
       _hasOutOfMemoryException(false),
       _binDataProto(_context),
       _bsonProto(_context),
@@ -749,6 +750,7 @@ void MozJSImplScope::reset() {
     unregisterOperation();
     _pendingKill.store(false);
     _pendingGC.store(false);
+    _requireOwnedObjects = false;
     advanceGeneration();
 }
 
@@ -869,6 +871,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 bfff70a93b0..edfc236e90c 100644
--- a/src/mongo/scripting/mozjs/implscope.h
+++ b/src/mongo/scripting/mozjs/implscope.h
@@ -305,6 +305,10 @@ public:
 
     void advanceGeneration() override;
 
+    void requireOwnedObjects() override;
+
+    bool requiresOwnedObjects() const;
+
     JS::HandleId getInternedStringId(InternedString name) {
         return _internedStrings.getInternedString(name);
     }
@@ -373,6 +377,7 @@ private:
     bool _quickExit;
     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 bb4cd5c06ff..9bb11ce400b 100644
--- a/src/mongo/scripting/mozjs/proxyscope.cpp
+++ b/src/mongo/scripting/mozjs/proxyscope.cpp
@@ -120,6 +120,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;