Bug 1465860 - Don't crash in JS IPC on invalid object id. r=evilpie
authorAlex Gaynor <agaynor@mozilla.com>
Thu, 31 May 2018 16:29:03 -0400
changeset 421291 6f29a4a9da707b28058a425749a7d7335fe5ae68
parent 421290 aadcedf5a07e2a80ad1763a88084bfec34b8ad5c
child 421292 d2202116d877960d828b06d43705186d5f3d31cd
push id34091
push userbtara@mozilla.com
push dateTue, 05 Jun 2018 13:52:34 +0000
treeherdermozilla-central@752465b44c79 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersevilpie
bugs1465860
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1465860 - Don't crash in JS IPC on invalid object id. r=evilpie Instead, return an error up to the caller, who can return an IPC error, which will kill the child. This is significantly friendlier to fuzzing. MozReview-Commit-ID: C67xSqUeN1i
js/ipc/JavaScriptBase.h
js/ipc/JavaScriptLogging.h
js/ipc/JavaScriptShared.h
js/ipc/WrapperOwner.cpp
--- a/js/ipc/JavaScriptBase.h
+++ b/js/ipc/JavaScriptBase.h
@@ -26,166 +26,188 @@ class JavaScriptBase : public WrapperOwn
 
     virtual void ActorDestroy(WrapperOwner::ActorDestroyReason why) override {
         WrapperOwner::ActorDestroy(why);
     }
 
     /*** IPC handlers ***/
 
     mozilla::ipc::IPCResult RecvPreventExtensions(const uint64_t& objId, ReturnStatus* rs) override {
-        if (!Answer::RecvPreventExtensions(ObjectId::deserialize(objId), rs)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvPreventExtensions(obj.value(), rs)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
     mozilla::ipc::IPCResult RecvGetPropertyDescriptor(const uint64_t& objId, const JSIDVariant& id,
                                                       ReturnStatus* rs,
                                                       PPropertyDescriptor* out) override {
-        if (!Answer::RecvGetPropertyDescriptor(ObjectId::deserialize(objId), id, rs, out)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvGetPropertyDescriptor(obj.value(), id, rs, out)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
     mozilla::ipc::IPCResult RecvGetOwnPropertyDescriptor(const uint64_t& objId,
                                                          const JSIDVariant& id,
                                                          ReturnStatus* rs,
                                                          PPropertyDescriptor* out) override {
-        if (!Answer::RecvGetOwnPropertyDescriptor(ObjectId::deserialize(objId), id, rs, out)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvGetOwnPropertyDescriptor(obj.value(), id, rs, out)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
     mozilla::ipc::IPCResult RecvDefineProperty(const uint64_t& objId, const JSIDVariant& id,
                                                const PPropertyDescriptor& flags, ReturnStatus* rs) override {
-        if (!Answer::RecvDefineProperty(ObjectId::deserialize(objId), id, flags, rs)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvDefineProperty(obj.value(), id, flags, rs)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
     mozilla::ipc::IPCResult RecvDelete(const uint64_t& objId, const JSIDVariant& id,
                                        ReturnStatus* rs) override {
-        if (!Answer::RecvDelete(ObjectId::deserialize(objId), id, rs)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvDelete(obj.value(), id, rs)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
 
     mozilla::ipc::IPCResult RecvHas(const uint64_t& objId, const JSIDVariant& id,
                                     ReturnStatus* rs, bool* bp) override {
-        if (!Answer::RecvHas(ObjectId::deserialize(objId), id, rs, bp)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvHas(obj.value(), id, rs, bp)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
     mozilla::ipc::IPCResult RecvHasOwn(const uint64_t& objId, const JSIDVariant& id,
                                        ReturnStatus* rs, bool* bp) override {
-        if (!Answer::RecvHasOwn(ObjectId::deserialize(objId), id, rs, bp)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvHasOwn(obj.value(), id, rs, bp)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
     mozilla::ipc::IPCResult RecvGet(const uint64_t& objId, const JSVariant& receiverVar, const JSIDVariant& id,
                                     ReturnStatus* rs, JSVariant* result) override {
-        if (!Answer::RecvGet(ObjectId::deserialize(objId), receiverVar, id, rs, result)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvGet(obj.value(), receiverVar, id, rs, result)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
     mozilla::ipc::IPCResult RecvSet(const uint64_t& objId, const JSIDVariant& id, const JSVariant& value,
                                     const JSVariant& receiverVar, ReturnStatus* rs) override {
-        if (!Answer::RecvSet(ObjectId::deserialize(objId), id, value, receiverVar, rs)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvSet(obj.value(), id, value, receiverVar, rs)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
 
     mozilla::ipc::IPCResult RecvIsExtensible(const uint64_t& objId, ReturnStatus* rs,
                                              bool* result) override {
-        if (!Answer::RecvIsExtensible(ObjectId::deserialize(objId), rs, result)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvIsExtensible(obj.value(), rs, result)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
     mozilla::ipc::IPCResult RecvCallOrConstruct(const uint64_t& objId, InfallibleTArray<JSParam>&& argv,
                                                 const bool& construct, ReturnStatus* rs, JSVariant* result,
                                                 nsTArray<JSParam>* outparams) override {
-        if (!Answer::RecvCallOrConstruct(ObjectId::deserialize(objId), std::move(argv), construct, rs, result, outparams)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvCallOrConstruct(obj.value(), std::move(argv), construct, rs, result, outparams)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
     mozilla::ipc::IPCResult RecvHasInstance(const uint64_t& objId, const JSVariant& v, ReturnStatus* rs, bool* bp) override {
-        if (!Answer::RecvHasInstance(ObjectId::deserialize(objId), v, rs, bp)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvHasInstance(obj.value(), v, rs, bp)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
     mozilla::ipc::IPCResult RecvGetBuiltinClass(const uint64_t& objId, ReturnStatus* rs, uint32_t* classValue) override {
-        if (!Answer::RecvGetBuiltinClass(ObjectId::deserialize(objId), rs, classValue)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvGetBuiltinClass(obj.value(), rs, classValue)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
     mozilla::ipc::IPCResult RecvIsArray(const uint64_t& objId, ReturnStatus* rs, uint32_t* answer) override {
-        if (!Answer::RecvIsArray(ObjectId::deserialize(objId), rs, answer)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvIsArray(obj.value(), rs, answer)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
     mozilla::ipc::IPCResult RecvClassName(const uint64_t& objId, nsCString* result) override {
-        if (!Answer::RecvClassName(ObjectId::deserialize(objId), result)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvClassName(obj.value(), result)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
     mozilla::ipc::IPCResult RecvGetPrototype(const uint64_t& objId, ReturnStatus* rs, ObjectOrNullVariant* result) override {
-        if (!Answer::RecvGetPrototype(ObjectId::deserialize(objId), rs, result)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvGetPrototype(obj.value(), rs, result)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
     mozilla::ipc::IPCResult RecvGetPrototypeIfOrdinary(const uint64_t& objId, ReturnStatus* rs, bool* isOrdinary,
                                                        ObjectOrNullVariant* result) override
     {
-        if (!Answer::RecvGetPrototypeIfOrdinary(ObjectId::deserialize(objId), rs, isOrdinary, result)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvGetPrototypeIfOrdinary(obj.value(), rs, isOrdinary, result)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
     mozilla::ipc::IPCResult RecvRegExpToShared(const uint64_t& objId, ReturnStatus* rs, nsString* source, uint32_t* flags) override {
-        if (!Answer::RecvRegExpToShared(ObjectId::deserialize(objId), rs, source, flags)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvRegExpToShared(obj.value(), rs, source, flags)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
 
     mozilla::ipc::IPCResult RecvGetPropertyKeys(const uint64_t& objId, const uint32_t& flags,
                                                 ReturnStatus* rs, nsTArray<JSIDVariant>* ids) override {
-        if (!Answer::RecvGetPropertyKeys(ObjectId::deserialize(objId), flags, rs, ids)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvGetPropertyKeys(obj.value(), flags, rs, ids)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
     mozilla::ipc::IPCResult RecvInstanceOf(const uint64_t& objId, const JSIID& iid,
                                            ReturnStatus* rs, bool* instanceof) override {
-        if (!Answer::RecvInstanceOf(ObjectId::deserialize(objId), iid, rs, instanceof)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvInstanceOf(obj.value(), iid, rs, instanceof)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
     mozilla::ipc::IPCResult RecvDOMInstanceOf(const uint64_t& objId, const int& prototypeID, const int& depth,
                                               ReturnStatus* rs, bool* instanceof) override {
-        if (!Answer::RecvDOMInstanceOf(ObjectId::deserialize(objId), prototypeID, depth, rs, instanceof)) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvDOMInstanceOf(obj.value(), prototypeID, depth, rs, instanceof)) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
 
     mozilla::ipc::IPCResult RecvDropObject(const uint64_t& objId) override {
-        if (!Answer::RecvDropObject(ObjectId::deserialize(objId))) {
+        Maybe<ObjectId> obj(ObjectId::deserialize(objId));
+        if (obj.isNothing() || !Answer::RecvDropObject(obj.value())) {
             return IPC_FAIL_NO_REASON(this);
         }
         return IPC_OK();
     }
 
     /*** Dummy call handlers ***/
 
     bool SendDropObject(const ObjectId& objId) override {
--- a/js/ipc/JavaScriptLogging.h
+++ b/js/ipc/JavaScriptLogging.h
@@ -163,20 +163,25 @@ class Logging
           case JSVariant::TnsString: {
               nsAutoCString tmp;
               format(value.get_nsString(), tmp);
               out = nsPrintfCString("\"%s\"", tmp.get());
               break;
           }
           case JSVariant::TObjectVariant: {
               const ObjectVariant& ovar = value.get_ObjectVariant();
-              if (ovar.type() == ObjectVariant::TLocalObject)
-                  formatObject(incoming, true, ObjectId::deserialize(ovar.get_LocalObject().serializedId()), out);
-              else
-                  formatObject(incoming, false, ObjectId::deserialize(ovar.get_RemoteObject().serializedId()), out);
+              if (ovar.type() == ObjectVariant::TLocalObject) {
+                  Maybe<ObjectId> objId(ObjectId::deserialize(ovar.get_LocalObject().serializedId()));
+                  MOZ_RELEASE_ASSERT(objId.isSome());
+                  formatObject(incoming, true, objId.value(), out);
+              } else {
+                  Maybe<ObjectId> objId(ObjectId::deserialize(ovar.get_RemoteObject().serializedId()));
+                  MOZ_RELEASE_ASSERT(objId.isSome());
+                  formatObject(incoming, false, objId.value(), out);
+              }
               break;
           }
           case JSVariant::TSymbolVariant: {
               out = "<Symbol>";
               break;
           }
           case JSVariant::Tdouble: {
               out = nsPrintfCString("%.0f", value.get_double());
--- a/js/ipc/JavaScriptShared.h
+++ b/js/ipc/JavaScriptShared.h
@@ -24,17 +24,17 @@ class ObjectId {
     // doubles. See bug 1065811 comment 12 for an explanation.
     static const size_t SERIAL_NUMBER_BITS = 47;
     static const size_t FLAG_BITS = 1;
     static const uint64_t SERIAL_NUMBER_MAX = (uint64_t(1) << SERIAL_NUMBER_BITS) - 1;
 
     explicit ObjectId(uint64_t serialNumber, bool hasXrayWaiver)
       : serialNumber_(serialNumber), hasXrayWaiver_(hasXrayWaiver)
     {
-        if (MOZ_UNLIKELY(serialNumber == 0 || serialNumber > SERIAL_NUMBER_MAX))
+        if (isInvalidSerialNumber(serialNumber))
             MOZ_CRASH("Bad CPOW Id");
     }
 
     bool operator==(const ObjectId& other) const {
         bool equal = serialNumber() == other.serialNumber();
         MOZ_ASSERT_IF(equal, hasXrayWaiver() == other.hasXrayWaiver());
         return equal;
     }
@@ -44,27 +44,34 @@ class ObjectId {
     uint64_t serialNumber() const { return serialNumber_; }
     bool hasXrayWaiver() const { return hasXrayWaiver_; }
     uint64_t serialize() const {
         MOZ_ASSERT(serialNumber(), "Don't send a null ObjectId over IPC");
         return uint64_t((serialNumber() << FLAG_BITS) | ((hasXrayWaiver() ? 1 : 0) << 0));
     }
 
     static ObjectId nullId() { return ObjectId(); }
-    static ObjectId deserialize(uint64_t data) {
-        return ObjectId(data >> FLAG_BITS, data & 1);
+    static Maybe<ObjectId> deserialize(uint64_t data) {
+        if (isInvalidSerialNumber(data >> FLAG_BITS)) {
+            return Nothing();
+        }
+        return Some(ObjectId(data >> FLAG_BITS, data & 1));
     }
 
     // For use with StructGCPolicy.
     void trace(JSTracer*) const {}
     bool needsSweep() const { return false; }
 
   private:
     ObjectId() : serialNumber_(0), hasXrayWaiver_(false) {}
 
+    static bool isInvalidSerialNumber(uint64_t aSerialNumber) {
+        return aSerialNumber == 0 || aSerialNumber > SERIAL_NUMBER_MAX;
+    }
+
     uint64_t serialNumber_ : SERIAL_NUMBER_BITS;
     bool hasXrayWaiver_ : 1;
 };
 
 class JavaScriptShared;
 
 // DefaultHasher<T> requires that T coerce to an integral type. We could make
 // ObjectId do that, but doing so would weaken our type invariants, so we just
--- a/js/ipc/WrapperOwner.cpp
+++ b/js/ipc/WrapperOwner.cpp
@@ -1176,17 +1176,19 @@ WrapperOwner::fromObjectVariant(JSContex
     } else {
         return fromLocalObjectVariant(cx, objVar.get_LocalObject());
     }
 }
 
 JSObject*
 WrapperOwner::fromRemoteObjectVariant(JSContext* cx, const RemoteObject& objVar)
 {
-    ObjectId objId = ObjectId::deserialize(objVar.serializedId());
+    Maybe<ObjectId> maybeObjId(ObjectId::deserialize(objVar.serializedId()));
+    MOZ_RELEASE_ASSERT(maybeObjId.isSome());
+    ObjectId objId = maybeObjId.value();
     RootedObject obj(cx, findCPOWById(objId));
     if (!obj) {
 
         // All CPOWs live in the privileged junk scope.
         RootedObject junkScope(cx, xpc::PrivilegedJunkScope());
         JSAutoRealm ar(cx, junkScope);
         RootedValue v(cx, UndefinedValue());
         // We need to setLazyProto for the getPrototype/getPrototypeIfOrdinary
@@ -1222,16 +1224,17 @@ WrapperOwner::fromRemoteObjectVariant(JS
     if (!JS_WrapObject(cx, &obj))
         return nullptr;
     return obj;
 }
 
 JSObject*
 WrapperOwner::fromLocalObjectVariant(JSContext* cx, const LocalObject& objVar)
 {
-    ObjectId id = ObjectId::deserialize(objVar.serializedId());
-    Rooted<JSObject*> obj(cx, findObjectById(cx, id));
+    Maybe<ObjectId> id(ObjectId::deserialize(objVar.serializedId()));
+    MOZ_RELEASE_ASSERT(id.isSome());
+    Rooted<JSObject*> obj(cx, findObjectById(cx, id.value()));
     if (!obj)
         return nullptr;
     if (!JS_WrapObject(cx, &obj))
         return nullptr;
     return obj;
 }