From f7b02cecab65040d38c053e4ddb58eb0a72b2f40 Mon Sep 17 00:00:00 2001 From: adeshkp Date: Sat, 12 Sep 2020 04:08:43 -0400 Subject: [PATCH 1/7] Issue #1280 - Remove hostname parameter to trust domain. Host name was purely being used for HPKP and since HPKP is killed, this can also go. Currently it doesn't do anything other than generating build warnings. --- security/certverifier/CertVerifier.cpp | 23 ++++++++----------- .../certverifier/NSSCertDBTrustDomain.cpp | 4 +--- security/certverifier/NSSCertDBTrustDomain.h | 4 +--- 3 files changed, 12 insertions(+), 19 deletions(-) diff --git a/security/certverifier/CertVerifier.cpp b/security/certverifier/CertVerifier.cpp index 1139ecae58..7f47de14fc 100644 --- a/security/certverifier/CertVerifier.cpp +++ b/security/certverifier/CertVerifier.cpp @@ -422,7 +422,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, SHA1Mode::Allowed, NetscapeStepUpPolicy::NeverMatch, originAttributes, - builtChain, nullptr); + builtChain); rv = BuildCertChain(trustDomain, certDER, time, EndEntityOrCA::MustBeEndEntity, KeyUsage::digitalSignature, @@ -489,8 +489,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, mCertShortLifetimeInDays, mPinningMode, MIN_RSA_BITS, ValidityCheckingMode::CheckForEV, sha1ModeConfigurations[i], mNetscapeStepUpPolicy, - originAttributes, builtChain, - hostname); + originAttributes, builtChain); rv = BuildCertChainForOneKeyUsage(trustDomain, certDER, time, KeyUsage::digitalSignature,// (EC)DHE KeyUsage::keyEncipherment, // RSA @@ -572,8 +571,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, ValidityCheckingMode::CheckingOff, sha1ModeConfigurations[j], mNetscapeStepUpPolicy, - originAttributes, builtChain, - hostname); + originAttributes, builtChain); rv = BuildCertChainForOneKeyUsage(trustDomain, certDER, time, KeyUsage::digitalSignature,//(EC)DHE KeyUsage::keyEncipherment,//RSA @@ -635,7 +633,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, pinningDisabled, MIN_RSA_BITS_WEAK, ValidityCheckingMode::CheckingOff, SHA1Mode::Allowed, mNetscapeStepUpPolicy, - originAttributes, builtChain, nullptr); + originAttributes, builtChain); rv = BuildCertChain(trustDomain, certDER, time, EndEntityOrCA::MustBeCA, KeyUsage::keyCertSign, KeyPurposeId::id_kp_serverAuth, @@ -651,7 +649,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, ValidityCheckingMode::CheckingOff, SHA1Mode::Allowed, NetscapeStepUpPolicy::NeverMatch, - originAttributes, builtChain, nullptr); + originAttributes, builtChain); rv = BuildCertChain(trustDomain, certDER, time, EndEntityOrCA::MustBeEndEntity, KeyUsage::digitalSignature, @@ -678,7 +676,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, ValidityCheckingMode::CheckingOff, SHA1Mode::Allowed, NetscapeStepUpPolicy::NeverMatch, - originAttributes, builtChain, nullptr); + originAttributes, builtChain); rv = BuildCertChain(trustDomain, certDER, time, EndEntityOrCA::MustBeEndEntity, KeyUsage::keyEncipherment, // RSA @@ -702,7 +700,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, ValidityCheckingMode::CheckingOff, SHA1Mode::Allowed, NetscapeStepUpPolicy::NeverMatch, - originAttributes, builtChain, nullptr); + originAttributes, builtChain); rv = BuildCertChain(trustDomain, certDER, time, EndEntityOrCA::MustBeEndEntity, KeyUsage::digitalSignature, @@ -735,7 +733,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, ValidityCheckingMode::CheckingOff, SHA1Mode::Allowed, NetscapeStepUpPolicy::NeverMatch, - originAttributes, builtChain, nullptr); + originAttributes, builtChain); rv = BuildCertChain(sslTrust, certDER, time, endEntityOrCA, keyUsage, eku, CertPolicyId::anyPolicy, stapledOCSPResponse); @@ -747,7 +745,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, ValidityCheckingMode::CheckingOff, SHA1Mode::Allowed, NetscapeStepUpPolicy::NeverMatch, - originAttributes, builtChain, nullptr); + originAttributes, builtChain); rv = BuildCertChain(emailTrust, certDER, time, endEntityOrCA, keyUsage, eku, CertPolicyId::anyPolicy, stapledOCSPResponse); @@ -761,8 +759,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, ValidityCheckingMode::CheckingOff, SHA1Mode::Allowed, NetscapeStepUpPolicy::NeverMatch, - originAttributes, builtChain, - nullptr); + originAttributes, builtChain); rv = BuildCertChain(objectSigningTrust, certDER, time, endEntityOrCA, keyUsage, eku, CertPolicyId::anyPolicy, stapledOCSPResponse); diff --git a/security/certverifier/NSSCertDBTrustDomain.cpp b/security/certverifier/NSSCertDBTrustDomain.cpp index fff75ee88f..a8ecbf1d11 100644 --- a/security/certverifier/NSSCertDBTrustDomain.cpp +++ b/security/certverifier/NSSCertDBTrustDomain.cpp @@ -58,8 +58,7 @@ NSSCertDBTrustDomain::NSSCertDBTrustDomain(SECTrustType certDBTrustType, CertVerifier::SHA1Mode sha1Mode, NetscapeStepUpPolicy netscapeStepUpPolicy, const NeckoOriginAttributes& originAttributes, - UniqueCERTCertList& builtChain, - /*optional*/ const char* hostname) + UniqueCERTCertList& builtChain) : mCertDBTrustType(certDBTrustType) , mOCSPFetching(ocspFetching) , mOCSPCache(ocspCache) @@ -73,7 +72,6 @@ NSSCertDBTrustDomain::NSSCertDBTrustDomain(SECTrustType certDBTrustType, , mNetscapeStepUpPolicy(netscapeStepUpPolicy) , mOriginAttributes(originAttributes) , mBuiltChain(builtChain) - , mHostname(hostname) , mCertBlocklist(do_GetService(NS_CERTBLOCKLIST_CONTRACTID)) , mOCSPStaplingStatus(CertVerifier::OCSP_STAPLING_NEVER_CHECKED) , mSCTListFromCertificate() diff --git a/security/certverifier/NSSCertDBTrustDomain.h b/security/certverifier/NSSCertDBTrustDomain.h index becf29eee9..50e2c8adc6 100644 --- a/security/certverifier/NSSCertDBTrustDomain.h +++ b/security/certverifier/NSSCertDBTrustDomain.h @@ -83,8 +83,7 @@ public: CertVerifier::SHA1Mode sha1Mode, NetscapeStepUpPolicy netscapeStepUpPolicy, const NeckoOriginAttributes& originAttributes, - UniqueCERTCertList& builtChain, - /*optional*/ const char* hostname = nullptr); + UniqueCERTCertList& builtChain); virtual Result FindIssuer(mozilla::pkix::Input encodedIssuerName, IssuerChecker& checker, @@ -187,7 +186,6 @@ private: NetscapeStepUpPolicy mNetscapeStepUpPolicy; const NeckoOriginAttributes& mOriginAttributes; UniqueCERTCertList& mBuiltChain; // non-owning - const char* mHostname; // non-owning - only used for pinning checks nsCOMPtr mCertBlocklist; CertVerifier::OCSPStaplingStatus mOCSPStaplingStatus; // Certificate Transparency data extracted during certificate verification From dc12585a82fecf91ab3441b85cd018ed3853c13a Mon Sep 17 00:00:00 2001 From: Moonchild Date: Sat, 12 Sep 2020 20:20:02 +0200 Subject: [PATCH 2/7] Issue #1650 - Add null check. There are situations where nsCSSClipPathinstance->CreateClipPath(dt) returns null. We need to check for this before trying to use its functions. If there is no clip path, then always return "no hit". --- layout/svg/nsCSSClipPathInstance.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/layout/svg/nsCSSClipPathInstance.cpp b/layout/svg/nsCSSClipPathInstance.cpp index 01f7de248e..e923eaa0e7 100644 --- a/layout/svg/nsCSSClipPathInstance.cpp +++ b/layout/svg/nsCSSClipPathInstance.cpp @@ -59,7 +59,7 @@ nsCSSClipPathInstance::HitTestBasicShapeClip(nsIFrame* aFrame, RefPtr path = instance.CreateClipPath(drawTarget); float pixelRatio = float(nsPresContext::AppUnitsPerCSSPixel()) / aFrame->PresContext()->AppUnitsPerDevPixel(); - return path->ContainsPoint(ToPoint(aPoint) * pixelRatio, Matrix()); + return path ? path->ContainsPoint(ToPoint(aPoint) * pixelRatio, Matrix()) : false; } already_AddRefed From cbd06086ea488eaa0744ff45f67efc438429e2b7 Mon Sep 17 00:00:00 2001 From: adeshkp Date: Sun, 13 Sep 2020 13:39:40 +0530 Subject: [PATCH 3/7] Fix usage of a macro in a cocoa widget (#1649) * Issue #457 - Fix usage of a macro in a cocoa widget --- widget/cocoa/nsDragService.mm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/widget/cocoa/nsDragService.mm b/widget/cocoa/nsDragService.mm index b92db1b2a9..c4d8e6ff06 100644 --- a/widget/cocoa/nsDragService.mm +++ b/widget/cocoa/nsDragService.mm @@ -222,7 +222,7 @@ nsDragService::ConstructDragImage(nsIDOMNode* aDOMNode, bool nsDragService::IsValidType(NSString* availableType, bool allowFileURL) { - NS_OBJC_BEGIN_TRY_ABORT_BLOCK; + NS_OBJC_BEGIN_TRY_ABORT_BLOCK_RETURN; // Prevent exposing fileURL for non-fileURL type. // We need URL provided by dropped webloc file, but don't need file's URL. @@ -234,7 +234,7 @@ nsDragService::IsValidType(NSString* availableType, bool allowFileURL) return true; - NS_OBJC_END_TRY_ABORT_BLOCK(false); + NS_OBJC_END_TRY_ABORT_BLOCK_RETURN(false); } NSString* From f04339dd5a3c4c62f0e3fe0a6c570631ccf0baf0 Mon Sep 17 00:00:00 2001 From: athenian200 Date: Sun, 6 Sep 2020 13:51:08 -0500 Subject: [PATCH 4/7] Issue #1629 - Part 4: Ensure isExplicitlyEnabled is false upon sheet creation. This clarifies the assumptions the code is making and the order in which the variables pass through the loading process. The new variable is set after the sheet is created and prepared, and is assumed to be false in the beginning. --- layout/style/Loader.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/layout/style/Loader.cpp b/layout/style/Loader.cpp index c211992381..5ca319b3f5 100644 --- a/layout/style/Loader.cpp +++ b/layout/style/Loader.cpp @@ -2273,7 +2273,8 @@ Loader::LoadChildSheet(StyleSheet* aParentSheet, state = eSheetComplete; } else { bool isAlternate; - bool isExplicitlyEnabled; + // For now, child sheets are not explicitly enabled. + bool isExplicitlyEnabled = false; const nsSubstring& empty = EmptyString(); // For now, use CORS_NONE for child sheets rv = CreateSheet(aURL, nullptr, principal, @@ -2397,7 +2398,9 @@ Loader::InternalLoadNonDocumentSheet(nsIURI* aURL, StyleSheetState state; bool isAlternate; - bool isExplicitlyEnabled; + // Sheets can only be explicitly enabled after creation and preparation, so + // set isExplicitlyEnabled to false. + bool isExplicitlyEnabled = false; RefPtr sheet; bool syncLoad = (aObserver == nullptr); const nsSubstring& empty = EmptyString(); From aa7b7fe713f3ee2512c9b4209954ca69c723ef84 Mon Sep 17 00:00:00 2001 From: athenian200 Date: Wed, 9 Sep 2020 01:40:15 -0500 Subject: [PATCH 5/7] Issue #1629 - Part 5: Remove pointless local variables. Since the local variable is always initialized to false, we don't actually need to declare it and can just pass "false" directly as a parameter to the PrepareSheet function's bool. I was worried about code readability at first, but some well-placed comments took care of that. --- layout/style/Loader.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/layout/style/Loader.cpp b/layout/style/Loader.cpp index 5ca319b3f5..155ecbeeef 100644 --- a/layout/style/Loader.cpp +++ b/layout/style/Loader.cpp @@ -2273,8 +2273,6 @@ Loader::LoadChildSheet(StyleSheet* aParentSheet, state = eSheetComplete; } else { bool isAlternate; - // For now, child sheets are not explicitly enabled. - bool isExplicitlyEnabled = false; const nsSubstring& empty = EmptyString(); // For now, use CORS_NONE for child sheets rv = CreateSheet(aURL, nullptr, principal, @@ -2284,8 +2282,9 @@ Loader::LoadChildSheet(StyleSheet* aParentSheet, parentData ? parentData->mSyncLoad : false, false, empty, state, &isAlternate, &sheet); NS_ENSURE_SUCCESS(rv, rv); - - PrepareSheet(sheet, empty, empty, aMedia, nullptr, isAlternate, isExplicitlyEnabled); + // For now, child sheets are not explicitly enabled (seventh argument is + // always false here). + PrepareSheet(sheet, empty, empty, aMedia, nullptr, isAlternate, false); } rv = InsertChildSheet(sheet, aParentSheet, aParentRule); @@ -2398,9 +2397,6 @@ Loader::InternalLoadNonDocumentSheet(nsIURI* aURL, StyleSheetState state; bool isAlternate; - // Sheets can only be explicitly enabled after creation and preparation, so - // set isExplicitlyEnabled to false. - bool isExplicitlyEnabled = false; RefPtr sheet; bool syncLoad = (aObserver == nullptr); const nsSubstring& empty = EmptyString(); @@ -2410,7 +2406,10 @@ Loader::InternalLoadNonDocumentSheet(nsIURI* aURL, false, empty, state, &isAlternate, &sheet); NS_ENSURE_SUCCESS(rv, rv); - PrepareSheet(sheet, empty, empty, nullptr, nullptr, isAlternate, isExplicitlyEnabled); + // Sheets can only be explicitly enabled after creation and preparation, so + // we always pass false for the initial value of the explicitly enabled flag + // when calling PrepareSheet. + PrepareSheet(sheet, empty, empty, nullptr, nullptr, isAlternate, false); if (state == eSheetComplete) { LOG((" Sheet already complete")); From 48006dc0755fc0e1800066c3986d735b26450cd5 Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Sun, 13 Sep 2020 19:29:23 -0400 Subject: [PATCH 6/7] Issue #618 - Clear the module map - Debug Followup An assertion was added to dom/script/ScriptLoader.cpp but fails to compile on debug builds due it not being included in the namespace. Ref: BZ 1529203 --- xpcom/glue/nsBaseHashtable.h | 1 + 1 file changed, 1 insertion(+) diff --git a/xpcom/glue/nsBaseHashtable.h b/xpcom/glue/nsBaseHashtable.h index f52df3dd18..f074037f39 100644 --- a/xpcom/glue/nsBaseHashtable.h +++ b/xpcom/glue/nsBaseHashtable.h @@ -59,6 +59,7 @@ public: typedef nsBaseHashtableET EntryType; using nsTHashtable::Contains; + using nsTHashtable::IsEmpty; nsBaseHashtable() {} explicit nsBaseHashtable(uint32_t aInitLength) From 7704d9e52297be741b98a5380bed510c69f63503 Mon Sep 17 00:00:00 2001 From: adeshkp Date: Sat, 12 Sep 2020 16:01:48 +0530 Subject: [PATCH 7/7] [Pale-Moon] [minor] spelling correction in a code comment --- application/palemoon/components/nsBrowserGlue.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/palemoon/components/nsBrowserGlue.js b/application/palemoon/components/nsBrowserGlue.js index e9bd019236..d93963721b 100644 --- a/application/palemoon/components/nsBrowserGlue.js +++ b/application/palemoon/components/nsBrowserGlue.js @@ -452,7 +452,7 @@ BrowserGlue.prototype = { Services.obs.notifyObservers(null, "browser-ui-startup-complete", ""); }, - // Copies additional profile files from the default profile tho the current profile. + // Copies additional profile files from the default profile to the current profile. // Only files not covered by the regular profile creation process. // Currently only the userchrome examples. _copyDefaultProfileFiles: function()