From 788cc4acb23fe4493f27aeee05223ad2de874a35 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Sat, 22 Mar 2025 20:42:50 +0100 Subject: [PATCH 1/4] No issue - Add range checking to SharedArrayBuffer references. This avoids a potential crash when juggling SAB allocations. --- js/src/js.msg | 1 + js/src/shell/js.cpp | 54 ++++++++++++++++++++------------- js/src/vm/SharedArrayObject.cpp | 18 +++++++++-- js/src/vm/SharedArrayObject.h | 2 +- js/src/vm/StructuredClone.cpp | 7 +++-- 5 files changed, 56 insertions(+), 26 deletions(-) diff --git a/js/src/js.msg b/js/src/js.msg index f21a371eff..5125cb12b8 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -445,6 +445,7 @@ MSG_DEF(JSMSG_SC_UNSUPPORTED_TYPE, 0, JSEXN_TYPEERR, "unsupported type for s 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") MSG_DEF(JSMSG_SC_SAB_DISABLED, 0, JSEXN_TYPEERR, "SharedArrayBuffer not cloned - shared memory disabled in receiver") +MSG_DEF(JSMSG_SC_SAB_TOO_MANY_REFS, 0, JSEXN_TYPEERR, "SharedArrayBuffer has too many references") // Debugger MSG_DEF(JSMSG_ASSIGN_FUNCTION_OR_NULL, 1, JSEXN_TYPEERR, "value assigned to {0} must be a function or null") diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index 387c78849f..6dd903e20b 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -5347,25 +5347,31 @@ GetSharedArrayBuffer(JSContext* cx, unsigned argc, Value* vp) { CallArgs args = CallArgsFromVp(argc, vp); JSObject* newObj = nullptr; - bool rval = true; - sharedArrayBufferMailboxLock->lock(); - SharedArrayRawBuffer* buf = sharedArrayBufferMailbox; - if (buf) { - buf->addReference(); - // Shared memory is enabled globally in the shell: there can't be a worker - // that does not enable it if the main thread has it. - MOZ_ASSERT(cx->compartment()->creationOptions().getSharedMemoryAndAtomicsEnabled()); - newObj = SharedArrayBufferObject::New(cx, buf); - if (!newObj) { - buf->dropReference(); - rval = false; + { + sharedArrayBufferMailboxLock->lock(); + auto unlockMailbox = MakeScopeExit([]() { sharedArrayBufferMailboxLock->unlock(); }); + + SharedArrayRawBuffer* buf = sharedArrayBufferMailbox; + if (buf) { + if (!buf->addReference()) { + JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_SC_SAB_REFCNT_OFLO); + return false; + } + + // Shared memory is enabled globally in the shell: there can't be a worker + // that does not enable it if the main thread has it. + MOZ_ASSERT(cx->compartment()->creationOptions().getSharedMemoryAndAtomicsEnabled()); + newObj = SharedArrayBufferObject::New(cx, buf); + if (!newObj) { + buf->dropReference(); + return false; + } } } - sharedArrayBufferMailboxLock->unlock(); args.rval().setObjectOrNull(newObj); - return rval; + return true; } static bool @@ -5379,18 +5385,24 @@ SetSharedArrayBuffer(JSContext* cx, unsigned argc, Value* vp) } else if (args.get(0).isObject() && args[0].toObject().is()) { newBuffer = args[0].toObject().as().rawBufferObject(); - newBuffer->addReference(); + if (!newBuffer->addReference()) { + JS_ReportErrorASCII(cx, "Reference count overflow on SharedArrayBuffer"); + return false; + } } else { JS_ReportErrorASCII(cx, "Only a SharedArrayBuffer can be installed in the global mailbox"); return false; } - sharedArrayBufferMailboxLock->lock(); - SharedArrayRawBuffer* oldBuffer = sharedArrayBufferMailbox; - if (oldBuffer) - oldBuffer->dropReference(); - sharedArrayBufferMailbox = newBuffer; - sharedArrayBufferMailboxLock->unlock(); + { + sharedArrayBufferMailboxLock->lock(); + auto unlockMailbox = MakeScopeExit([]() { sharedArrayBufferMailboxLock->unlock(); }); + + SharedArrayRawBuffer* oldBuffer = sharedArrayBufferMailbox; + if (oldBuffer) + oldBuffer->dropReference(); + sharedArrayBufferMailbox = newBuffer; + } args.rval().setUndefined(); return true; diff --git a/js/src/vm/SharedArrayObject.cpp b/js/src/vm/SharedArrayObject.cpp index 44fe3b790d..6a3c6a91c3 100644 --- a/js/src/vm/SharedArrayObject.cpp +++ b/js/src/vm/SharedArrayObject.cpp @@ -166,16 +166,30 @@ SharedArrayRawBuffer::New(JSContext* cx, uint32_t length) return rawbuf; } -void +bool SharedArrayRawBuffer::addReference() { MOZ_ASSERT(this->refcount_ > 0); - ++this->refcount_; // Atomic. + + // Be careful never to overflow the refcount field. + for (;;) { + uint32_t old_refcount = this->refcount_; + uint32_t new_refcount = old_refcount+1; + if (new_refcount == 0) + return false; + if (this->refcount_.compareExchange(old_refcount, new_refcount)) + return true; + } } void SharedArrayRawBuffer::dropReference() { + // Normally if the refcount is zero then the memory will have been unmapped + // and this test may just crash, but if the memory has been retained for any + // reason we will catch the underflow here. + MOZ_RELEASE_ASSERT(this->refcount_ > 0); + // Drop the reference to the buffer. uint32_t refcount = --this->refcount_; // Atomic. if (refcount) diff --git a/js/src/vm/SharedArrayObject.h b/js/src/vm/SharedArrayObject.h index 19048336cb..d6b18b61a3 100644 --- a/js/src/vm/SharedArrayObject.h +++ b/js/src/vm/SharedArrayObject.h @@ -91,7 +91,7 @@ class SharedArrayRawBuffer uint32_t refcount() const { return refcount_; } - void addReference(); + [[nodiscard]] bool addReference(); void dropReference(); }; diff --git a/js/src/vm/StructuredClone.cpp b/js/src/vm/StructuredClone.cpp index 79f2d23d96..e1f1b2777e 100644 --- a/js/src/vm/StructuredClone.cpp +++ b/js/src/vm/StructuredClone.cpp @@ -1201,9 +1201,12 @@ JSStructuredCloneWriter::writeSharedArrayBuffer(HandleObject obj) Rooted sharedArrayBuffer(context(), &CheckedUnwrap(obj)->as()); SharedArrayRawBuffer* rawbuf = sharedArrayBuffer->rawBufferObject(); - // Avoids a race condition where the parent thread frees the buffer + // Adding the reference here avoids a race condition where the parent thread frees the buffer // before the child has accepted the transferable. - rawbuf->addReference(); + if (!rawbuf->addReference()) { + JS_ReportErrorNumberASCII(context(), GetErrorMessage, nullptr, JSMSG_SC_SAB_TOO_MANY_REFS); + return false; + } intptr_t p = reinterpret_cast(rawbuf); return out.writePair(SCTAG_SHARED_ARRAY_BUFFER_OBJECT, static_cast(sizeof(p))) && From c31cda6c742f997fd3258c76261bbf79a7543378 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Tue, 25 Mar 2025 00:10:53 +0100 Subject: [PATCH 2/4] No issue - Fix incorrect js message ref in js shell. --- js/src/shell/js.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index 6dd903e20b..2c3d7f7a1d 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -5355,7 +5355,7 @@ GetSharedArrayBuffer(JSContext* cx, unsigned argc, Value* vp) SharedArrayRawBuffer* buf = sharedArrayBufferMailbox; if (buf) { if (!buf->addReference()) { - JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_SC_SAB_REFCNT_OFLO); + JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_SC_SAB_TOO_MANY_REFS); return false; } From 7d3dc5a2f25b168b4212d75cac1b89b0f5371d42 Mon Sep 17 00:00:00 2001 From: Martok Date: Sun, 23 Mar 2025 16:32:54 +0100 Subject: [PATCH 3/4] Issue #2692 - Follow-Up: fix compilation in debug --- js/src/builtin/TestingFunctions.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/builtin/TestingFunctions.cpp b/js/src/builtin/TestingFunctions.cpp index f8ac4fd22e..e5b2954130 100644 --- a/js/src/builtin/TestingFunctions.cpp +++ b/js/src/builtin/TestingFunctions.cpp @@ -3946,7 +3946,7 @@ ParseRegExp(JSContext* cx, unsigned argc, Value* vp) if (!pattern) return false; - CompileOptions options(cx); + JS::CompileOptions options(cx); frontend::TokenStream dummyTokenStream(cx, options, nullptr, 0, nullptr); irregexp::RegExpCompileData data; From 46fadb2d2b611b0c32843ffabedef0f8d565cd3a Mon Sep 17 00:00:00 2001 From: Martok Date: Tue, 25 Mar 2025 22:39:06 +0100 Subject: [PATCH 4/4] Issue #2715 - Compute Channel Binding Hashes using the certificate signature's hash algorithm as per spec --- extensions/auth/nsAuthSSPI.cpp | 200 ++++++++++++++++++++++++++++++--- 1 file changed, 183 insertions(+), 17 deletions(-) diff --git a/extensions/auth/nsAuthSSPI.cpp b/extensions/auth/nsAuthSSPI.cpp index 5649dc25f9..8dce3e4e81 100644 --- a/extensions/auth/nsAuthSSPI.cpp +++ b/extensions/auth/nsAuthSSPI.cpp @@ -19,6 +19,11 @@ #include "nsNetCID.h" #include "nsCOMPtr.h" #include "nsICryptoHash.h" +#include "nsIX509Cert.h" +#include "nsNSSCertificate.h" + +#include "ScopedNSSTypes.h" +#include "secoid.h" #include @@ -103,7 +108,7 @@ MakeSN(const char *principal, nsCString &result) int32_t index = buf.FindChar('@'); if (index == kNotFound) return NS_ERROR_UNEXPECTED; - + nsCOMPtr dns = do_GetService(NS_DNSSERVICE_CONTRACTID, &rv); if (NS_FAILED(rv)) return rv; @@ -134,6 +139,160 @@ MakeSN(const char *principal, nsCString &result) //----------------------------------------------------------------------------- +/* + * This is pulled from the switch statement in sec_DecodeSigAlg in security/nss/lib/cryptohi/secvfy.c, + * which is not exported to the public API. + */ +SECOidTag +DecodeSigAlg(const SECKEYPublicKey *key, SECOidTag sigAlg) +{ + unsigned int len; + + switch (sigAlg) { + // Old RSA + case SEC_OID_PKCS1_MD2_WITH_RSA_ENCRYPTION: + return SEC_OID_MD2; + case SEC_OID_PKCS1_MD4_WITH_RSA_ENCRYPTION: + return SEC_OID_MD4; + case SEC_OID_PKCS1_MD5_WITH_RSA_ENCRYPTION: + return SEC_OID_MD5; + case SEC_OID_PKCS1_SHA1_WITH_RSA_ENCRYPTION: + case SEC_OID_ISO_SHA_WITH_RSA_SIGNATURE: + case SEC_OID_ISO_SHA1_WITH_RSA_SIGNATURE: + return SEC_OID_SHA1; + case SEC_OID_PKCS1_RSA_ENCRYPTION: + // could be estimated from RSA signature + return SEC_OID_UNKNOWN; + case SEC_OID_PKCS1_RSA_PSS_SIGNATURE: + // default, SHA-1 + return SEC_OID_SHA1; + + // Newer RSA and ECDSA + case SEC_OID_ANSIX962_ECDSA_SHA224_SIGNATURE: + case SEC_OID_PKCS1_SHA224_WITH_RSA_ENCRYPTION: + case SEC_OID_NIST_DSA_SIGNATURE_WITH_SHA224_DIGEST: + return SEC_OID_SHA224; + case SEC_OID_ANSIX962_ECDSA_SHA256_SIGNATURE: + case SEC_OID_PKCS1_SHA256_WITH_RSA_ENCRYPTION: + case SEC_OID_NIST_DSA_SIGNATURE_WITH_SHA256_DIGEST: + return SEC_OID_SHA256; + case SEC_OID_ANSIX962_ECDSA_SHA384_SIGNATURE: + case SEC_OID_PKCS1_SHA384_WITH_RSA_ENCRYPTION: + return SEC_OID_SHA384; + case SEC_OID_ANSIX962_ECDSA_SHA512_SIGNATURE: + case SEC_OID_PKCS1_SHA512_WITH_RSA_ENCRYPTION: + return SEC_OID_SHA512; + + // DSA signatures + case SEC_OID_ANSIX9_DSA_SIGNATURE_WITH_SHA1_DIGEST: + case SEC_OID_BOGUS_DSA_SIGNATURE_WITH_SHA1_DIGEST: + case SEC_OID_ANSIX962_ECDSA_SHA1_SIGNATURE: + return SEC_OID_SHA1; + case SEC_OID_MISSI_DSS: + case SEC_OID_MISSI_KEA_DSS: + case SEC_OID_MISSI_KEA_DSS_OLD: + case SEC_OID_MISSI_DSS_OLD: + return SEC_OID_SHA1; + case SEC_OID_ANSIX962_ECDSA_SIGNATURE_RECOMMENDED_DIGEST: + /* This is an EC algorithm. Recommended means the largest + * hash algorithm that is not reduced by the keysize of + * the EC algorithm. Note that key strength is in bytes and + * algorithms are specified in bits. Never use an algorithm + * weaker than sha1. */ + len = SECKEY_PublicKeyStrength(key); + if (len < 28) { /* 28 bytes == 224 bits */ + return SEC_OID_SHA1; + } + if (len < 32) { /* 32 bytes == 256 bits */ + return SEC_OID_SHA224; + } + if (len < 48) { /* 48 bytes == 384 bits */ + return SEC_OID_SHA256; + } + if (len < 64) { /* 64 bytes == 512 bits */ + return SEC_OID_SHA384; + } + /* use the largest in this case */ + return SEC_OID_SHA512; + case SEC_OID_ANSIX962_ECDSA_SIGNATURE_SPECIFIED_DIGEST: + // would need to parse params to resolve this + return SEC_OID_UNKNOWN; + default: + return SEC_OID_UNKNOWN; + } +} + +//----------------------------------------------------------------------------- + +uint32_t +CertSignatureHashFunction(const uint8_t *certDER, uint32_t certDERLen) +{ + // Get the certificate object from DER encoding + nsCOMPtr x509 = nsNSSCertificate::ConstructFromDER((char*)certDER, certDERLen); + if (!x509) + return 0; + + mozilla::UniqueCERTCertificate cert(x509->GetCert()); + if (!cert) + return 0; + + // Get the signature and public key material (needed for some EC signatures) + SECAlgorithmID *algID = &cert->signature; + SECOidTag sigAlg = SECOID_FindOIDTag(&algID->algorithm); + + CERTSubjectPublicKeyInfo *spki = &cert->subjectPublicKeyInfo; + mozilla::UniqueSECKEYPublicKey pubKey(SECKEY_ExtractPublicKey(spki)); + + // Translate signature OID to hash algorithm OID + SECOidTag hashAlg = DecodeSigAlg(pubKey.get(), sigAlg); + + // Return the nsICryptoHash to use for a certificate signed with the given hash algorithm OID + switch (hashAlg) { + // Newer hashes, must use the type as used in the cert itself + case SEC_OID_SHA224: + return nsICryptoHash::SHA224; + case SEC_OID_SHA256: + return nsICryptoHash::SHA256; + case SEC_OID_SHA384: + return nsICryptoHash::SHA384; + case SEC_OID_SHA512: + return nsICryptoHash::SHA512; + // SHA-1 and MD must use SHA-256, do that for fallback too + case SEC_OID_MD2: + case SEC_OID_MD4: + case SEC_OID_MD5: + case SEC_OID_SHA1: + default: + return nsICryptoHash::SHA256; + } +} + +//----------------------------------------------------------------------------- + +uint32_t +DigestByteSize(uint32_t nsICryptoHashAlgo) +{ + switch(nsICryptoHashAlgo) { + case nsICryptoHash::MD2: + return 16; + case nsICryptoHash::MD5: + return 16; + case nsICryptoHash::SHA1: + return 20; + case nsICryptoHash::SHA256: + return 32; + case nsICryptoHash::SHA384: + return 48; + case nsICryptoHash::SHA512: + return 64; + case nsICryptoHash::SHA224: + return 28; + } + return 0; +} + +//----------------------------------------------------------------------------- + nsAuthSSPI::nsAuthSSPI(pType package) : mServiceFlags(REQ_DEFAULT) , mMaxTokenLen(0) @@ -287,11 +446,9 @@ nsAuthSSPI::GetNextToken(const void *inToken, uint32_t *outTokenLen) { // String for end-point bindings. - const char end_point[] = "tls-server-end-point:"; + const char end_point[] = "tls-server-end-point:"; const int end_point_length = sizeof(end_point) - 1; - const int hash_size = 32; // Size of a SHA256 hash. - const int cbt_size = hash_size + end_point_length; - + SECURITY_STATUS rc; MS_TimeStamp ignored; @@ -345,11 +502,21 @@ nsAuthSSPI::GetNextToken(const void *inToken, ibd.ulVersion = SECBUFFER_VERSION; ibd.cBuffers = 0; ibd.pBuffers = ib; - + // If we have stored a certificate, the Channel Binding Token // needs to be generated and sent in the first input buffer. if (mCertDERLength > 0) { - // First we create a proper Endpoint Binding structure. + // We need to find out what signature algorithm is used in + // the certificate's signature and how long its digest is. + uint32_t hashFunc = CertSignatureHashFunction((unsigned char*)mCertDERData, mCertDERLength); + if (!hashFunc) + return NS_ERROR_FAILURE; + const int hash_size = DigestByteSize(hashFunc); + if (!hash_size) + return NS_ERROR_FAILURE; + const int cbt_size = hash_size + end_point_length; + + // First we create a proper Endpoint Binding structure. pendpoint_binding.dwInitiatorAddrType = 0; pendpoint_binding.cbInitiatorLength = 0; pendpoint_binding.dwInitiatorOffset = 0; @@ -357,7 +524,7 @@ nsAuthSSPI::GetNextToken(const void *inToken, pendpoint_binding.cbAcceptorLength = 0; pendpoint_binding.dwAcceptorOffset = 0; pendpoint_binding.cbApplicationDataLength = cbt_size; - pendpoint_binding.dwApplicationDataOffset = + pendpoint_binding.dwApplicationDataOffset = sizeof(SEC_CHANNEL_BINDINGS); // Then add it to the array of sec buffers accordingly. @@ -365,7 +532,7 @@ nsAuthSSPI::GetNextToken(const void *inToken, ib[ibd.cBuffers].cbBuffer = pendpoint_binding.cbApplicationDataLength + pendpoint_binding.dwApplicationDataOffset; - + sspi_cbt = (char *) moz_xmalloc(ib[ibd.cBuffers].cbBuffer); if (!sspi_cbt){ return NS_ERROR_OUT_OF_MEMORY; @@ -373,7 +540,7 @@ nsAuthSSPI::GetNextToken(const void *inToken, // Helper to write in the memory block that stores the CBT char* sspi_cbt_ptr = sspi_cbt; - + ib[ibd.cBuffers].pvBuffer = sspi_cbt; ibd.cBuffers++; @@ -383,16 +550,14 @@ nsAuthSSPI::GetNextToken(const void *inToken, memcpy(sspi_cbt_ptr, end_point, end_point_length); sspi_cbt_ptr += end_point_length; - - // Start hashing. We are always doing SHA256, but depending - // on the certificate, a different alogirthm might be needed. - nsAutoCString hashString; + // Start hashing. + nsAutoCString hashString; nsresult rv; nsCOMPtr crypto; crypto = do_CreateInstance(NS_CRYPTO_HASH_CONTRACTID, &rv); if (NS_SUCCEEDED(rv)) - rv = crypto->Init(nsICryptoHash::SHA256); + rv = crypto->Init(hashFunc); if (NS_SUCCEEDED(rv)) rv = crypto->Update((unsigned char*)mCertDERData, mCertDERLength); if (NS_SUCCEEDED(rv)) @@ -404,12 +569,13 @@ nsAuthSSPI::GetNextToken(const void *inToken, free(sspi_cbt); return rv; } - + // Once the hash has been computed, we store it in memory right // after the Endpoint structure and the "tls-server-end-point:" // char array. + MOZ_ASSERT(hashString.Length() == hash_size); memcpy(sspi_cbt_ptr, hashString.get(), hash_size); - + // Free memory used to store the server certificate free(mCertDERData); mCertDERData = nullptr;