From 2497cc338e4feeca78e310ae468446558d66524f Mon Sep 17 00:00:00 2001 From: win7-7 Date: Sat, 27 Dec 2025 13:38:12 +0200 Subject: [PATCH] Stability fixes backporting. 1438645 - Prevent XDR from reading past its buffer., 1325345 - Add compartment asserts to JS_FireOnNewGlobalObject and other APIs. 1339411 - Rewrite and optimize object allocation paths, NatibeObject.h, NativeObject-inl.h and jsiter.cpp were not updated use JS::Result for stability, do that now. --- js/src/jsapi.cpp | 15 ++++++++++ js/src/jsatom.cpp | 18 +++++++++--- js/src/jsfriendapi.cpp | 9 ++++++ js/src/jsiter.cpp | 6 ++-- js/src/shell/js.cpp | 6 ++-- js/src/vm/GlobalObject.cpp | 5 ++-- js/src/vm/NativeObject-inl.h | 6 ++-- js/src/vm/NativeObject.h | 6 ++-- js/src/vm/Xdr.cpp | 7 +++-- js/src/vm/Xdr.h | 56 +++++++++++++++++++++++++----------- 10 files changed, 96 insertions(+), 38 deletions(-) diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index da30e0414e..57eec17bd5 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -161,6 +161,7 @@ JS::ObjectOpResult::reportStrictErrorOrWarning(JSContext* cx, HandleObject obj, "unsigned value of OkCode must not be an error code"); MOZ_ASSERT(code_ != Uninitialized); MOZ_ASSERT(!ok()); + assertSameCompartment(cx, obj); unsigned flags = strict ? JSREPORT_ERROR : (JSREPORT_WARNING | JSREPORT_STRICT); if (code_ == JSMSG_OBJECT_NOT_EXTENSIBLE || code_ == JSMSG_SET_NON_OBJECT_RECEIVER) { @@ -195,6 +196,7 @@ JS::ObjectOpResult::reportStrictErrorOrWarning(JSContext* cx, HandleObject obj, MOZ_ASSERT(code_ != Uninitialized); MOZ_ASSERT(!ok()); MOZ_ASSERT(!ErrorTakesArguments(code_)); + assertSameCompartment(cx, obj); unsigned flags = strict ? JSREPORT_ERROR : (JSREPORT_WARNING | JSREPORT_STRICT); return JS_ReportErrorFlagsAndNumberASCII(cx, flags, GetErrorMessage, nullptr, code_); @@ -1582,6 +1584,7 @@ JS::ToPrimitive(JSContext* cx, HandleObject obj, JSType hint, MutableHandleValue { AssertHeapIsIdle(cx); CHECK_REQUEST(cx); + assertSameCompartment(cx, obj); MOZ_ASSERT(obj != nullptr); MOZ_ASSERT(hint == JSTYPE_VOID || hint == JSTYPE_STRING || hint == JSTYPE_NUMBER); vp.setObject(*obj); @@ -1861,6 +1864,7 @@ JS_FireOnNewGlobalObject(JSContext* cx, JS::HandleObject global) // to be able to throw errors during delicate global creation routines. // This infallibility will eat OOM and slow script, but if that happens // we'll likely run up into them again soon in a fallible context. + assertSameCompartment(cx, global); Rooted globalObject(cx, &global->as()); Debugger::onNewGlobalObject(cx, globalObject); } @@ -1941,6 +1945,7 @@ JS::AssertObjectBelongsToCurrentThread(JSObject* obj) JS_PUBLIC_API(bool) JS_GetPrototype(JSContext* cx, HandleObject obj, MutableHandleObject result) { + assertSameCompartment(cx, obj); return GetPrototype(cx, obj, result); } @@ -1958,24 +1963,28 @@ JS_PUBLIC_API(bool) JS_GetPrototypeIfOrdinary(JSContext* cx, HandleObject obj, bool* isOrdinary, MutableHandleObject result) { + assertSameCompartment(cx, obj); return GetPrototypeIfOrdinary(cx, obj, isOrdinary, result); } JS_PUBLIC_API(bool) JS_IsExtensible(JSContext* cx, HandleObject obj, bool* extensible) { + assertSameCompartment(cx, obj); return IsExtensible(cx, obj, extensible); } JS_PUBLIC_API(bool) JS_PreventExtensions(JSContext* cx, JS::HandleObject obj, ObjectOpResult& result) { + assertSameCompartment(cx, obj); return PreventExtensions(cx, obj, result); } JS_PUBLIC_API(bool) JS_SetImmutablePrototype(JSContext *cx, JS::HandleObject obj, bool *succeeded) { + assertSameCompartment(cx, obj); return SetImmutablePrototype(cx, obj, succeeded); } @@ -1985,6 +1994,7 @@ JS_GetOwnPropertyDescriptorById(JSContext* cx, HandleObject obj, HandleId id, { AssertHeapIsIdle(cx); CHECK_REQUEST(cx); + assertSameCompartment(cx, obj, id); return GetOwnPropertyDescriptor(cx, obj, id, desc); } @@ -2428,6 +2438,7 @@ static bool DefineElement(JSContext* cx, HandleObject obj, uint32_t index, HandleValue value, unsigned attrs, Native getter, Native setter) { + assertSameCompartment(cx, obj, value); AutoRooterGetterSetter gsRoot(cx, attrs, &getter, &setter); AssertHeapIsIdle(cx); CHECK_REQUEST(cx); @@ -2494,6 +2505,7 @@ JS_HasPropertyById(JSContext* cx, HandleObject obj, HandleId id, bool* foundp) { AssertHeapIsIdle(cx); CHECK_REQUEST(cx); + assertSameCompartment(cx, obj, id); return HasProperty(cx, obj, id, foundp); } @@ -3233,6 +3245,9 @@ JS::ObjectToCompletePropertyDescriptor(JSContext* cx, HandleValue descObj, MutableHandle desc) { + // |obj| can be in a different compartment here. The caller is responsible + // for wrapping it (see JS_WrapPropertyDescriptor). + assertSameCompartment(cx, descObj); if (!ToPropertyDescriptor(cx, descObj, true, desc)) return false; CompletePropertyDescriptor(desc); diff --git a/js/src/jsatom.cpp b/js/src/jsatom.cpp index 2a72ac38a3..522d0f44f8 100644 --- a/js/src/jsatom.cpp +++ b/js/src/jsatom.cpp @@ -551,15 +551,25 @@ js::XDRAtom(XDRState* xdr, MutableHandleAtom atomp) JSAtom* atom; if (latin1) { const Latin1Char* chars = nullptr; - if (length) - chars = reinterpret_cast(xdr->buf.read(length)); + if (length) { + const uint8_t *ptr; + size_t nbyte = length * sizeof(Latin1Char); + if (!xdr->peekData(&ptr, nbyte)) + return false; + chars = reinterpret_cast(ptr); + } atom = AtomizeChars(cx, chars, length); } else { #if MOZ_LITTLE_ENDIAN /* Directly access the little endian chars in the XDR buffer. */ const char16_t* chars = nullptr; - if (length) - chars = reinterpret_cast(xdr->buf.read(length * sizeof(char16_t))); + if (length) { + const uint8_t *ptr; + size_t nbyte = length * sizeof(char16_t); + if (!xdr->peekData(&ptr, nbyte)) + return false; + chars = reinterpret_cast(ptr); + } atom = AtomizeChars(cx, chars, length); #else /* diff --git a/js/src/jsfriendapi.cpp b/js/src/jsfriendapi.cpp index f73cc5fd65..d027f3aa53 100644 --- a/js/src/jsfriendapi.cpp +++ b/js/src/jsfriendapi.cpp @@ -70,6 +70,7 @@ JS_SetGrayGCRootsTracer(JSContext* cx, JSTraceDataOp traceOp, void* data) JS_FRIEND_API(JSObject*) JS_FindCompilationScope(JSContext* cx, HandleObject objArg) { + assertSameCompartment(cx, objArg); RootedObject obj(cx, objArg); /* @@ -103,6 +104,7 @@ JS_SplicePrototype(JSContext* cx, HandleObject obj, HandleObject proto) * does not nuke type information for the object. */ CHECK_REQUEST(cx); + assertSameCompartment(cx, obj, proto); if (!obj->isSingleton()) { /* @@ -137,6 +139,7 @@ JS_NewObjectWithUniqueType(JSContext* cx, const JSClass* clasp, HandleObject pro JS_FRIEND_API(JSObject*) JS_NewObjectWithoutMetadata(JSContext* cx, const JSClass* clasp, JS::Handle proto) { + assertSameCompartment(cx, proto); AutoSuppressAllocationMetadataBuilder suppressMetadata(cx); return JS_NewObjectWithGivenProto(cx, clasp, proto); } @@ -312,6 +315,7 @@ js::GetBuiltinClass(JSContext* cx, HandleObject obj, ESClass* cls) JS_FRIEND_API(const char*) js::ObjectClassName(JSContext* cx, HandleObject obj) { + assertSameCompartment(cx, obj); return GetObjectClassName(cx, obj); } @@ -512,6 +516,8 @@ js::FunctionHasNativeReserved(JSObject* fun) JS_FRIEND_API(bool) js::GetObjectProto(JSContext* cx, JS::Handle obj, JS::MutableHandle proto) { + assertSameCompartment(cx, obj); + if (IsProxy(obj)) return JS_GetPrototype(cx, obj, proto); @@ -648,6 +654,8 @@ js::StringToLinearStringSlow(JSContext* cx, JSString* str) JS_FRIEND_API(JSObject*) JS_CloneObject(JSContext* cx, HandleObject obj, HandleObject protoArg) { + // |obj| might be in a different compartment. + assertSameCompartment(cx, protoArg); Rooted proto(cx, TaggedProto(protoArg.get())); return CloneObject(cx, obj, proto); } @@ -1343,6 +1351,7 @@ js::MaybeGetScriptPrivate(JSObject* object) { JS_FRIEND_API(bool) js::ReportIsNotFunction(JSContext* cx, HandleValue v) { + assertSameCompartment(cx, v); return ReportIsNotFunction(cx, v, -1); } diff --git a/js/src/jsiter.cpp b/js/src/jsiter.cpp index 726a84985a..740009d9fe 100644 --- a/js/src/jsiter.cpp +++ b/js/src/jsiter.cpp @@ -950,9 +950,9 @@ js::CreateIterResultObject(JSContext* cx, HandleValue value, bool done) if (!templateObject) return nullptr; - NativeObject* resultObj = NativeObject::createWithTemplate(cx, gc::DefaultHeap, templateObject); - if (!resultObj) - return nullptr; + NativeObject* resultObj; + JS_TRY_VAR_OR_RETURN_NULL( + cx, resultObj, NativeObject::createWithTemplate(cx, gc::DefaultHeap, templateObject)); // Step 3. resultObj->setSlot(JSCompartment::IterResultObjectValueSlot, value); diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index ed30afa7ef..d526ea98a6 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -3355,10 +3355,9 @@ NewSandbox(JSContext* cx, bool lazy) RootedValue value(cx, BooleanValue(lazy)); if (!JS_SetProperty(cx, obj, "lazy", value)) return nullptr; + JS_FireOnNewGlobalObject(cx, obj); } - JS_FireOnNewGlobalObject(cx, obj); - if (!cx->compartment()->wrap(cx, &obj)) return nullptr; return obj; @@ -7576,10 +7575,9 @@ NewGlobalObject(JSContext* cx, JS::CompartmentOptions& options, /* Initialize FakeDOMObject.prototype */ InitDOMObject(domProto); + JS_FireOnNewGlobalObject(cx, glob); } - JS_FireOnNewGlobalObject(cx, glob); - return glob; } diff --git a/js/src/vm/GlobalObject.cpp b/js/src/vm/GlobalObject.cpp index b3080d6edd..62a6fef073 100644 --- a/js/src/vm/GlobalObject.cpp +++ b/js/src/vm/GlobalObject.cpp @@ -405,11 +405,10 @@ GlobalObject::new_(JSContext* cx, const Class* clasp, JSPrincipals* principals, global = GlobalObject::createInternal(cx, clasp); if (!global) return nullptr; + if (hookOption == JS::FireOnNewGlobalHook) + JS_FireOnNewGlobalObject(cx, global); } - if (hookOption == JS::FireOnNewGlobalHook) - JS_FireOnNewGlobalObject(cx, global); - return global; } diff --git a/js/src/vm/NativeObject-inl.h b/js/src/vm/NativeObject-inl.h index 1fa5cbd10e..bb93ff43da 100644 --- a/js/src/vm/NativeObject-inl.h +++ b/js/src/vm/NativeObject-inl.h @@ -250,7 +250,7 @@ NativeObject::getDenseOrTypedArrayElement(ExclusiveContext* cx, uint32_t idx, return true; } -/* static */ inline NativeObject* +/* static */ inline JS::Result NativeObject::createWithTemplate(JSContext* cx, gc::InitialHeap heap, HandleObject templateObject) { @@ -266,8 +266,8 @@ NativeObject::createWithTemplate(JSContext* cx, gc::InitialHeap heap, return &baseObj->as(); } -/* static */ inline NativeObject* -NativeObject::copy(ExclusiveContext* cx, gc::AllocKind kind, gc::InitialHeap heap, +/* static */ inline JS::Result +NativeObject::copy(JSContext* cx, gc::AllocKind kind, gc::InitialHeap heap, HandleNativeObject templateObject) { RootedShape shape(cx, templateObject->lastProperty()); diff --git a/js/src/vm/NativeObject.h b/js/src/vm/NativeObject.h index 8a24dffdf7..f57ff90e63 100644 --- a/js/src/vm/NativeObject.h +++ b/js/src/vm/NativeObject.h @@ -486,7 +486,7 @@ class NativeObject : public ShapedObject return cells && cells->hasCell(cell); } - static inline NativeObject* + static inline JS::Result createWithTemplate(JSContext* cx, js::gc::InitialHeap heap, HandleObject templateObject); protected: @@ -1321,8 +1321,8 @@ class NativeObject : public ShapedObject return privateRef(nfixed); } - static inline NativeObject* - copy(ExclusiveContext* cx, gc::AllocKind kind, gc::InitialHeap heap, + static inline JS::Result + copy(JSContext* cx, gc::AllocKind kind, gc::InitialHeap heap, HandleNativeObject templateObject); void updateShapeAfterMovingGC(); diff --git a/js/src/vm/Xdr.cpp b/js/src/vm/Xdr.cpp index 3b34973a11..88bfa99e73 100644 --- a/js/src/vm/Xdr.cpp +++ b/js/src/vm/Xdr.cpp @@ -29,8 +29,9 @@ template void XDRState::postProcessContextErrors(ExclusiveContext* cx) { - if (cx->isJSContext() && cx->asJSContext()->isExceptionPending()) { - MOZ_ASSERT(resultCode_ == JS::TranscodeResult_Ok); + if (!cx->helperThread() && cx->isExceptionPending()) { + MOZ_ASSERT(resultCode_ == JS::TranscodeResult_Ok || + resultCode_ == JS::TranscodeResult_Throw); resultCode_ = JS::TranscodeResult_Throw; } } @@ -67,6 +68,8 @@ XDRState::codeChars(char16_t* chars, size_t nchars) mozilla::NativeEndian::copyAndSwapToLittleEndian(ptr, chars, nchars); } else { const uint8_t* ptr = buf.read(nbytes); + if (!ptr) + return fail(JS::TranscodeResult_Failure_BadDecode); mozilla::NativeEndian::copyAndSwapFromLittleEndian(chars, ptr, nchars); } return true; diff --git a/js/src/vm/Xdr.h b/js/src/vm/Xdr.h index e0fe0ced45..4172b56a76 100644 --- a/js/src/vm/Xdr.h +++ b/js/src/vm/Xdr.h @@ -29,15 +29,10 @@ class XDRBuffer { MOZ_ASSERT(cursor_ < buffer_.length()); uint8_t* ptr = &buffer_[cursor_]; cursor_ += n; - return ptr; - } - - const char* readCString() { - char* ptr = reinterpret_cast(&buffer_[cursor_]); - uint8_t* end = reinterpret_cast(strchr(ptr, '\0')) + 1; - MOZ_ASSERT(buffer_.begin() < end); - MOZ_ASSERT(end <= buffer_.end()); - cursor_ = end - buffer_.begin(); + + // Don't let buggy code read past our buffer + if (cursor_ > buffer_.length()) + return nullptr; return ptr; } @@ -124,7 +119,7 @@ class XDRCoderBase template class XDRState : public XDRCoderBase { - public: + protected: XDRBuffer buf; private: JS::TranscodeResult resultCode_; @@ -163,6 +158,14 @@ class XDRState : public XDRCoderBase return false; } + bool peekData(const uint8_t** pptr, size_t length) { + const uint8_t* ptr = buf.read(length); + if (!ptr) + return fail(JS::TranscodeResult_Failure_BadDecode); + *pptr = ptr; + return true; + } + bool codeUint8(uint8_t* n) { if (mode == XDR_ENCODE) { uint8_t* ptr = buf.write(sizeof(*n)); @@ -170,7 +173,10 @@ class XDRState : public XDRCoderBase return fail(JS::TranscodeResult_Throw); *ptr = *n; } else { - *n = *buf.read(sizeof(*n)); + const uint8_t* ptr = buf.read(sizeof(*n)); + if (!ptr) + return fail(JS::TranscodeResult_Failure_BadDecode); + *n = *ptr; } return true; } @@ -183,6 +189,8 @@ class XDRState : public XDRCoderBase mozilla::LittleEndian::writeUint16(ptr, *n); } else { const uint8_t* ptr = buf.read(sizeof(*n)); + if (!ptr) + return fail(JS::TranscodeResult_Failure_BadDecode); *n = mozilla::LittleEndian::readUint16(ptr); } return true; @@ -196,6 +204,8 @@ class XDRState : public XDRCoderBase mozilla::LittleEndian::writeUint32(ptr, *n); } else { const uint8_t* ptr = buf.read(sizeof(*n)); + if (!ptr) + return fail(JS::TranscodeResult_Failure_BadDecode); *n = mozilla::LittleEndian::readUint32(ptr); } return true; @@ -209,6 +219,8 @@ class XDRState : public XDRCoderBase mozilla::LittleEndian::writeUint64(ptr, *n); } else { const uint8_t* ptr = buf.read(sizeof(*n)); + if (!ptr) + return fail(JS::TranscodeResult_Failure_BadDecode); *n = mozilla::LittleEndian::readUint64(ptr); } return true; @@ -271,7 +283,10 @@ class XDRState : public XDRCoderBase return fail(JS::TranscodeResult_Throw); memcpy(ptr, bytes, len); } else { - memcpy(bytes, buf.read(len), len); + const uint8_t* ptr = buf.read(len); + if (!ptr) + return fail(JS::TranscodeResult_Failure_BadDecode); + memcpy(bytes, ptr, len); } return true; } @@ -283,14 +298,23 @@ class XDRState : public XDRCoderBase * the decoding buffer. */ bool codeCString(const char** sp) { + uint64_t len64; + if (mode == XDR_ENCODE) + len64 = (uint64_t)(strlen(*sp) + 1); + if (!codeUint64(&len64)) + return false; + size_t len = (size_t) len64; + if (mode == XDR_ENCODE) { - size_t n = strlen(*sp) + 1; - uint8_t* ptr = buf.write(n); + uint8_t* ptr = buf.write(len); if (!ptr) return fail(JS::TranscodeResult_Throw); - memcpy(ptr, *sp, n); + memcpy(ptr, *sp, len); } else { - *sp = buf.readCString(); + const uint8_t* ptr = buf.read(len); + if (!ptr || ptr[len] != '\0') + return fail(JS::TranscodeResult_Failure_BadDecode); + *sp = reinterpret_cast(ptr); } return true; }