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.

9 files changed, 79 insertions, 3 deletions

diff --git a/src/mongo/db/commands/mr.cpp b/src/mongo/db/commands/mr.cpp
index bfcef8aa93f..28ac94f3b32 100644
--- a/src/mongo/db/commands/mr.cpp
+++ b/src/mongo/db/commands/mr.cpp
@@ -836,6 +836,7 @@ void State::init() {
     const string userToken =
         AuthorizationSession::get(Client::getCurrent())->getAuthenticatedUserNamesToken();
     _scope.reset(getGlobalScriptEngine()->newScopeForCurrentThread());
+    _scope->requireOwnedObjects();
     _scope->registerOperation(_opCtx);
     _scope->setLocalDB(_config.dbname);
     _scope->loadStored(_opCtx, true);
@@ -1502,6 +1503,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 fa117f08b61..6b1d2d27dd1 100644
--- a/src/mongo/dbtests/jstests.cpp
+++ b/src/mongo/dbtests/jstests.cpp
@@ -2368,6 +2368,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") {}
@@ -2424,6 +2467,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 69b680efe5a..e813e7771e7 100644
--- a/src/mongo/scripting/engine.cpp
+++ b/src/mongo/scripting/engine.cpp
@@ -440,6 +440,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 ff6dd1b1616..d3322a67e8e 100644
--- a/src/mongo/scripting/engine.h
+++ b/src/mongo/scripting/engine.h
@@ -102,6 +102,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 bf6c3e5ff65..55c376c1a19 100644
--- a/src/mongo/scripting/mozjs/implscope.cpp
+++ b/src/mongo/scripting/mozjs/implscope.cpp
@@ -408,6 +408,7 @@ MozJSImplScope::MozJSImplScope(MozJSScriptEngine* engine)
       _connectState(ConnectState::Not),
       _status(Status::OK()),
       _generation(0),
+      _requireOwnedObjects(false),
       _hasOutOfMemoryException(false),
       _binDataProto(_context),
       _bsonProto(_context),
@@ -843,6 +844,7 @@ void MozJSImplScope::reset() {
     unregisterOperation();
     _pendingKill.store(false);
     _pendingGC.store(false);
+    _requireOwnedObjects = false;
     advanceGeneration();
 }
 
@@ -957,6 +959,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 0e879280924..8ae4c6ab3d5 100644
--- a/src/mongo/scripting/mozjs/implscope.h
+++ b/src/mongo/scripting/mozjs/implscope.h
@@ -315,6 +315,10 @@ public:
 
     void advanceGeneration() override;
 
+    void requireOwnedObjects() override;
+
+    bool requiresOwnedObjects() const;
+
     JS::HandleId getInternedStringId(InternedString name) {
         return _internedStrings.getInternedString(name);
     }
@@ -411,6 +415,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 fec94a88a97..60e979bac74 100644
--- a/src/mongo/scripting/mozjs/proxyscope.cpp
+++ b/src/mongo/scripting/mozjs/proxyscope.cpp
@@ -123,6 +123,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 248e80a068c..9e779fa9ee9 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;