From c2e992ed6e5e07e4e2af94dfc815f88cf955c705 Mon Sep 17 00:00:00 2001 From: Tom Ritter Date: Thu, 19 Sep 2019 02:32:41 +0000 Subject: [PATCH] Bug 1570681 - Enforce eval restrictions in system contexts and the parent process r=ckerschb We log to MOZ_LOG, report an error to the console, send telemetry, and in debug builds - crash Differential Revision: https://phabricator.services.mozilla.com/D45055 --HG-- extra : moz-landing-system : lando --- caps/nsScriptSecurityManager.cpp | 8 +- .../en-US/chrome/security/security.properties | 2 + dom/security/CSPEvalChecker.cpp | 15 ++- dom/security/nsContentSecurityUtils.cpp | 117 ++++++++++++------ dom/security/nsContentSecurityUtils.h | 5 +- 5 files changed, 97 insertions(+), 50 deletions(-) mode change 100755 => 100644 dom/security/nsContentSecurityUtils.cpp diff --git a/caps/nsScriptSecurityManager.cpp b/caps/nsScriptSecurityManager.cpp index 28e3fc4cf669..8fa6b231b104 100644 --- a/caps/nsScriptSecurityManager.cpp +++ b/caps/nsScriptSecurityManager.cpp @@ -442,9 +442,11 @@ bool nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction( } } -#if !defined(ANDROID) && (defined(NIGHTLY_BUILD) || defined(DEBUG)) - nsContentSecurityUtils::AssertEvalNotRestricted(cx, subjectPrincipal, - scriptSample); +#if !defined(ANDROID) + if (!nsContentSecurityUtils::IsEvalAllowed(cx, subjectPrincipal, + scriptSample)) { + return false; + } #endif if (NS_FAILED(rv)) { diff --git a/dom/locales/en-US/chrome/security/security.properties b/dom/locales/en-US/chrome/security/security.properties index d1540859aea9..7c142d9fed02 100644 --- a/dom/locales/en-US/chrome/security/security.properties +++ b/dom/locales/en-US/chrome/security/security.properties @@ -101,6 +101,8 @@ BlockSubresourceRedirectToData=Redirecting to insecure data: URI not allowed (Bl BlockSubresourceFTP=Loading FTP subresource within http(s) page not allowed (Blocked loading of: “%1$S”) +RestrictBrowserEvalUsage=eval() and eval-like uses are not allowed in the Parent Process or in System Contexts (Blocked usage in “%1$S”) + # LOCALIZATION NOTE (BrowserUpgradeInsecureDisplayRequest): # %1$S is the browser name "brandShortName"; %2$S is the URL of the upgraded request; %1$S is the upgraded scheme. BrowserUpgradeInsecureDisplayRequest = %1$S is upgrading an insecure display request ‘%2$S’ to use ‘%3$S’ diff --git a/dom/security/CSPEvalChecker.cpp b/dom/security/CSPEvalChecker.cpp index c721c40c0d8a..706ef6032062 100644 --- a/dom/security/CSPEvalChecker.cpp +++ b/dom/security/CSPEvalChecker.cpp @@ -31,15 +31,18 @@ nsresult CheckInternal(nsIContentSecurityPolicy* aCSP, MOZ_ASSERT(NS_IsMainThread()); MOZ_ASSERT(aAllowed); -#if !defined(ANDROID) && (defined(NIGHTLY_BUILD) || defined(DEBUG)) - JSContext* cx = nsContentUtils::GetCurrentJSContext(); - nsContentSecurityUtils::AssertEvalNotRestricted(cx, aSubjectPrincipal, - aExpression); -#endif - // The value is set at any "return", but better to have a default value here. *aAllowed = false; +#if !defined(ANDROID) + JSContext* cx = nsContentUtils::GetCurrentJSContext(); + if (!nsContentSecurityUtils::IsEvalAllowed(cx, aSubjectPrincipal, + aExpression)) { + *aAllowed = false; + return NS_OK; + } +#endif + if (!aCSP) { *aAllowed = true; return NS_OK; diff --git a/dom/security/nsContentSecurityUtils.cpp b/dom/security/nsContentSecurityUtils.cpp old mode 100755 new mode 100644 index edb8dfc56ea2..8dce8cb77016 --- a/dom/security/nsContentSecurityUtils.cpp +++ b/dom/security/nsContentSecurityUtils.cpp @@ -202,8 +202,9 @@ FilenameType nsContentSecurityUtils::FilenameToEvalType( } /* static */ -void nsContentSecurityUtils::AssertEvalNotRestricted( - JSContext* cx, nsIPrincipal* aSubjectPrincipal, const nsAString& aScript) { +bool nsContentSecurityUtils::IsEvalAllowed(JSContext* cx, + nsIPrincipal* aSubjectPrincipal, + const nsAString& aScript) { // This allowlist contains files that are permanently allowed to use // eval()-like functions. It is supposed to be restricted to files that are // exclusively used in testing contexts. @@ -234,7 +235,7 @@ void nsContentSecurityUtils::AssertEvalNotRestricted( ("Allowing eval() %s because allowing pref is " "enabled", (systemPrincipal ? "with System Principal" : "in parent process"))); - return; + return true; } if (XRE_IsE10sParentProcess() && @@ -242,12 +243,12 @@ void nsContentSecurityUtils::AssertEvalNotRestricted( MOZ_LOG(sCSMLog, LogLevel::Debug, ("Allowing eval() in parent process because allowing pref is " "enabled")); - return; + return true; } if (!systemPrincipal && !XRE_IsE10sParentProcess()) { // Usage of eval we are unconcerned with. - return; + return true; } // This preference is a file used for autoconfiguration of Firefox @@ -262,7 +263,7 @@ void nsContentSecurityUtils::AssertEvalNotRestricted( ("Allowing eval() %s because of " "general.config.filename", (systemPrincipal ? "with System Principal" : "in parent process"))); - return; + return true; } // This preference is better known as userchrome.css which allows @@ -278,7 +279,7 @@ void nsContentSecurityUtils::AssertEvalNotRestricted( ("Allowing eval() %s because of " "toolkit.legacyUserProfileCustomizations.stylesheets", (systemPrincipal ? "with System Principal" : "in parent process"))); - return; + return true; } // We permit these two common idioms to get access to the global JS object @@ -289,31 +290,32 @@ void nsContentSecurityUtils::AssertEvalNotRestricted( ("Allowing eval() %s because a key string is " "provided", (systemPrincipal ? "with System Principal" : "in parent process"))); - return; + return true; } // Check the allowlist for the provided filename. getFilename is a helper // function - auto getFilename = [](JSContext* cx) -> nsCString { - JS::AutoFilename scriptFilename; - if (JS::DescribeScriptedCaller(cx, &scriptFilename)) { - nsDependentCSubstring fileName_(scriptFilename.get(), - strlen(scriptFilename.get())); - ToLowerCase(fileName_); - // Extract file name alone if scriptFilename contains line number - // separated by multiple space delimiters in few cases. - int32_t fileNameIndex = fileName_.FindChar(' '); - if (fileNameIndex != -1) { - fileName_.SetLength(fileNameIndex); - } - - nsAutoCString fileName(fileName_); - return std::move(fileName); + nsAutoCString fileName; + uint32_t lineNumber = 0, columnNumber = 0; + JS::AutoFilename rawScriptFilename; + if (JS::DescribeScriptedCaller(cx, &rawScriptFilename, &lineNumber, + &columnNumber)) { + nsDependentCSubstring fileName_(rawScriptFilename.get(), + strlen(rawScriptFilename.get())); + ToLowerCase(fileName_); + // Extract file name alone if scriptFilename contains line number + // separated by multiple space delimiters in few cases. + int32_t fileNameIndex = fileName_.FindChar(' '); + if (fileNameIndex != -1) { + fileName_.SetLength(fileNameIndex); } - return NS_LITERAL_CSTRING("unknown-file"); - }; - nsCString fileName = getFilename(cx); + fileName = std::move(fileName_); + } else { + fileName = NS_LITERAL_CSTRING("unknown-file"); + } + + NS_ConvertUTF8toUTF16 fileNameA(fileName); for (const nsLiteralCString& allowlistEntry : evalAllowlist) { if (fileName.Equals(allowlistEntry)) { MOZ_LOG( @@ -321,17 +323,23 @@ void nsContentSecurityUtils::AssertEvalNotRestricted( ("Allowing eval() %s because the containing " "file is in the allowlist", (systemPrincipal ? "with System Principal" : "in parent process"))); - return; + return true; } } + // Log to MOZ_LOG + MOZ_LOG(sCSMLog, LogLevel::Warning, + ("Blocking eval() %s from file %s and script " + "provided %s", + (systemPrincipal ? "with System Principal" : "in parent process"), + fileName.get(), NS_ConvertUTF16toUTF8(aScript).get())); + // Send Telemetry Telemetry::EventID eventType = systemPrincipal ? Telemetry::EventID::Security_Evalusage_Systemcontext : Telemetry::EventID::Security_Evalusage_Parentprocess; - FilenameType fileNameType = - FilenameToEvalType(NS_ConvertUTF8toUTF16(fileName)); + FilenameType fileNameType = FilenameToEvalType(fileNameA); mozilla::Maybe> extra; if (fileNameType.second().isSome()) { extra = Some>({EventExtraEntry{ @@ -346,23 +354,56 @@ void nsContentSecurityUtils::AssertEvalNotRestricted( } Telemetry::RecordEvent(eventType, mozilla::Some(fileNameType.first()), extra); - // Crash or Log + // Report an error to console + nsCOMPtr console( + do_GetService(NS_CONSOLESERVICE_CONTRACTID)); + if (!console) { + return false; + } + nsCOMPtr error(do_CreateInstance(NS_SCRIPTERROR_CONTRACTID)); + if (!error) { + return false; + } + nsCOMPtr bundle; + nsCOMPtr stringService = + mozilla::services::GetStringBundleService(); + if (!stringService) { + return false; + } + stringService->CreateBundle( + "chrome://global/locale/security/security.properties", + getter_AddRefs(bundle)); + if (!bundle) { + return false; + } + nsAutoString message; + AutoTArray formatStrings = {fileNameA}; + nsresult rv = bundle->FormatStringFromName("RestrictBrowserEvalUsage", + formatStrings, message); + if (NS_FAILED(rv)) { + return false; + } + + uint64_t windowID = nsJSUtils::GetCurrentlyRunningCodeInnerWindowID(cx); + rv = error->InitWithWindowID(message, fileNameA, EmptyString(), lineNumber, + columnNumber, nsIScriptError::errorFlag, + "BrowserEvalUsage", windowID, + true /* From chrome context */); + if (NS_FAILED(rv)) { + return false; + } + console->LogMessage(error); + + // Maybe Crash #ifdef DEBUG MOZ_CRASH_UNSAFE_PRINTF( "Blocking eval() %s from file %s and script provided " "%s", (systemPrincipal ? "with System Principal" : "in parent process"), fileName.get(), NS_ConvertUTF16toUTF8(aScript).get()); -#else - MOZ_LOG(sCSMLog, LogLevel::Warning, - ("Blocking eval() %s from file %s and script " - "provided %s", - (systemPrincipal ? "with System Principal" : "in parent process"), - fileName.get(), NS_ConvertUTF16toUTF8(aScript).get())); #endif - // In the future, we will change this function to return false and abort JS - // execution without crashing the process. For now, just collect data. + return false; } #if defined(DEBUG) diff --git a/dom/security/nsContentSecurityUtils.h b/dom/security/nsContentSecurityUtils.h index b4eab1e27492..9ab91abfaf6e 100644 --- a/dom/security/nsContentSecurityUtils.h +++ b/dom/security/nsContentSecurityUtils.h @@ -20,9 +20,8 @@ typedef mozilla::Pair> FilenameType; class nsContentSecurityUtils { public: static FilenameType FilenameToEvalType(const nsString& fileName); - static void AssertEvalNotRestricted(JSContext* cx, - nsIPrincipal* aSubjectPrincipal, - const nsAString& aScript); + static bool IsEvalAllowed(JSContext* cx, nsIPrincipal* aSubjectPrincipal, + const nsAString& aScript); #if defined(DEBUG) static void AssertAboutPageHasCSP(mozilla::dom::Document* aDocument);