From 9a6e3bc30d38640c5336a45299e97e54207b0168 Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Tue, 1 Oct 2024 22:22:04 +0200 Subject: [PATCH 1/2] [js] Disallow deserializing structured clone buffers with transferables more than once. --- js/public/StructuredClone.h | 1 + js/src/js.msg | 1 + js/src/vm/StructuredClone.cpp | 36 ++++++++++++++++++++++++++--------- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/js/public/StructuredClone.h b/js/public/StructuredClone.h index 59b68890af..2efbc7ad89 100644 --- a/js/public/StructuredClone.h +++ b/js/public/StructuredClone.h @@ -472,6 +472,7 @@ class JS_PUBLIC_API(JSAutoStructuredCloneBuffer) { #define JS_SCERR_TRANSFERABLE 1 #define JS_SCERR_DUP_TRANSFERABLE 2 #define JS_SCERR_UNSUPPORTED_TYPE 3 +#define JS_SCERR_TRANSFERABLE_TWICE 4 JS_PUBLIC_API(bool) JS_ReadUint32Pair(JSStructuredCloneReader* r, uint32_t* p1, uint32_t* p2); diff --git a/js/src/js.msg b/js/src/js.msg index 1fa7a65661..f21a371eff 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -440,6 +440,7 @@ MSG_DEF(JSMSG_SC_BAD_CLONE_VERSION, 0, JSEXN_ERR, "unsupported structured clo MSG_DEF(JSMSG_SC_BAD_SERIALIZED_DATA, 1, JSEXN_INTERNALERR, "bad serialized structured data ({0})") MSG_DEF(JSMSG_SC_DUP_TRANSFERABLE, 0, JSEXN_TYPEERR, "duplicate transferable for structured clone") MSG_DEF(JSMSG_SC_NOT_TRANSFERABLE, 0, JSEXN_TYPEERR, "invalid transferable array for structured clone") +MSG_DEF(JSMSG_SC_TRANSFERABLE_TWICE, 0, JSEXN_TYPEERR, "structured clone cannot transfer twice") MSG_DEF(JSMSG_SC_UNSUPPORTED_TYPE, 0, JSEXN_TYPEERR, "unsupported type for structured data") MSG_DEF(JSMSG_SC_NOT_CLONABLE, 1, JSEXN_TYPEERR, "{0} cannot be cloned in this context") MSG_DEF(JSMSG_SC_SAB_TRANSFER, 0, JSEXN_WARN, "SharedArrayBuffer must not be in the transfer list") diff --git a/js/src/vm/StructuredClone.cpp b/js/src/vm/StructuredClone.cpp index daaaf52b92..93f4fae626 100644 --- a/js/src/vm/StructuredClone.cpp +++ b/js/src/vm/StructuredClone.cpp @@ -140,18 +140,23 @@ enum StructuredDataType : uint32_t { /* * Format of transfer map: - * - * numTransferables (64 bits) - * array of: - * - * pointer (64 bits) - * extraData (64 bits), eg byte length for ArrayBuffers + * - + * - numTransferables (64 bits) + * - array of: + * - pointer (64 + * bits) + * - extraData (64 bits), eg byte length for ArrayBuffers + * - any data written for custom transferables */ // Data associated with an SCTAG_TRANSFER_MAP_HEADER that tells whether the -// contents have been read out yet or not. +// contents have been read out yet or not. TRANSFERRING is for the case where we +// have started but not completed reading, which due to errors could mean that +// there are things still owned by the clone buffer that need to be released, so +// discarding should not just be skipped. enum TransferableMapHeader { SCTAG_TM_UNREAD = 0, + SCTAG_TM_TRANSFERRING, SCTAG_TM_TRANSFERRED }; @@ -530,6 +535,10 @@ ReportDataCloneError(JSContext* cx, JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_SC_UNSUPPORTED_TYPE); break; + case JS_SCERR_TRANSFERABLE_TWICE: + JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_SC_TRANSFERABLE_TWICE); + break; + default: MOZ_CRASH("Unkown errorId"); break; @@ -2325,9 +2334,18 @@ JSStructuredCloneReader::readTransferMap() if (!in.getPair(&tag, &data)) return in.reportTruncated(); - if (tag != SCTAG_TRANSFER_MAP_HEADER || TransferableMapHeader(data) == SCTAG_TM_TRANSFERRED) + auto transferState = static_cast(data); + + if (tag != SCTAG_TRANSFER_MAP_HEADER || transferState == SCTAG_TM_TRANSFERRED) return true; + if (transferState == SCTAG_TM_TRANSFERRING) { + ReportDataCloneError(cx, callbacks, JS_SCERR_TRANSFERABLE_TWICE); + return false; + } + + headerPos.write(PairToUInt64(SCTAG_TRANSFER_MAP_HEADER, SCTAG_TM_TRANSFERRING)); + uint64_t numTransferables; MOZ_ALWAYS_TRUE(in.readPair(&tag, &data)); if (!in.read(&numTransferables)) @@ -2414,7 +2432,7 @@ JSStructuredCloneReader::readTransferMap() #ifdef DEBUG SCInput::getPair(headerPos.peek(), &tag, &data); MOZ_ASSERT(tag == SCTAG_TRANSFER_MAP_HEADER); - MOZ_ASSERT(TransferableMapHeader(data) != SCTAG_TM_TRANSFERRED); + MOZ_ASSERT(TransferableMapHeader(data) == SCTAG_TM_TRANSFERRING); #endif headerPos.write(PairToUInt64(SCTAG_TRANSFER_MAP_HEADER, SCTAG_TM_TRANSFERRED)); From b848a924bca8a89dd55644e463e56ba27b062618 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Wed, 2 Oct 2024 10:34:38 +0200 Subject: [PATCH 2/2] [GMP] Factor out more detailed CheckDimensions function for CreateFrame. --- dom/media/gmp/GMPVideoi420FrameImpl.cpp | 35 +++++++++++++++++++++---- dom/media/gmp/GMPVideoi420FrameImpl.h | 3 +++ 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/dom/media/gmp/GMPVideoi420FrameImpl.cpp b/dom/media/gmp/GMPVideoi420FrameImpl.cpp index fdbb9a9624..ed1f8cfea5 100644 --- a/dom/media/gmp/GMPVideoi420FrameImpl.cpp +++ b/dom/media/gmp/GMPVideoi420FrameImpl.cpp @@ -89,6 +89,34 @@ GMPVideoi420FrameImpl::CheckFrameData(const GMPVideoi420FrameData& aFrameData) return true; } +bool +GMPVideoi420FrameImpl::CheckDimensions(int32_t aWidth, int32_t aHeight, + int32_t aStride_y, + int32_t aStride_u, + int32_t aStride_v, int32_t aSize_y, + int32_t aSize_u, int32_t aSize_v) { + if (aWidth < 1 || aHeight < 1 || aStride_y < aWidth || aSize_y < 1 || + aSize_u < 1 || aSize_v < 1) { + return false; + } + auto halfWidth = (CheckedInt(aWidth) + 1) / 2; + if (!halfWidth.isValid() || aStride_u < halfWidth.value() || + aStride_v < halfWidth.value()) { + return false; + } + auto height = CheckedInt(aHeight); + auto halfHeight = (height + 1) / 2; + auto minSizeY = height * aStride_y; + auto minSizeU = halfHeight * aStride_u; + auto minSizeV = halfHeight * aStride_v; + if (!minSizeY.isValid() || !minSizeU.isValid() || !minSizeV.isValid() || + minSizeY.value() > aSize_y || minSizeU.value() > aSize_u || + minSizeV.value() > aSize_v) { + return false; + } + return true; +} + bool GMPVideoi420FrameImpl::CheckDimensions(int32_t aWidth, int32_t aHeight, int32_t aStride_y, int32_t aStride_u, int32_t aStride_v) @@ -179,11 +207,8 @@ GMPVideoi420FrameImpl::CreateFrame(int32_t aSize_y, const uint8_t* aBuffer_y, MOZ_ASSERT(aBuffer_u); MOZ_ASSERT(aBuffer_v); - if (aSize_y < 1 || aSize_u < 1 || aSize_v < 1) { - return GMPGenericErr; - } - - if (!CheckDimensions(aWidth, aHeight, aStride_y, aStride_u, aStride_v)) { + if (!CheckDimensions(aWidth, aHeight, aStride_y, aStride_u, aStride_v, + aSize_y, aSize_u, aSize_v)) { return GMPGenericErr; } diff --git a/dom/media/gmp/GMPVideoi420FrameImpl.h b/dom/media/gmp/GMPVideoi420FrameImpl.h index f5cb0254b9..700162255b 100644 --- a/dom/media/gmp/GMPVideoi420FrameImpl.h +++ b/dom/media/gmp/GMPVideoi420FrameImpl.h @@ -65,6 +65,9 @@ public: void ResetSize() override; private: + bool CheckDimensions(int32_t aWidth, int32_t aHeight, int32_t aStride_y, + int32_t aStride_u, int32_t aStride_v, int32_t aSize_y, + int32_t aSize_u, int32_t aSize_v); bool CheckDimensions(int32_t aWidth, int32_t aHeight, int32_t aStride_y, int32_t aStride_u, int32_t aStride_v);