From 42e3d7392303cb27481fedfc5cb82963f259c552 Mon Sep 17 00:00:00 2001 From: Joshua Bell Date: Fri, 17 Jan 2020 22:28:41 +0000 Subject: [Backport] CVE-2020-6399 - Insufficient policy enforcement in AppCache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Manual backport of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/1999300 https://chromium-review.googlesource.com/c/chromium/src/+/2007520: AppCache: Remove nonstandard "isPattern" support Chrome's AppCache implementation supported specifying namespaces as regular expressions that match URLs. This extension was invoked by adding the `isPattern` keyword after the namespace in the manifest. Histograms indicate that there is no usage of this feature. Start the removal process by removing parser support and having tests ensure the parser treats such entries normally. Subsequent CLs will delete the plumbing entirely. (cherry picked from commit 034b02983e7b849eab657fcdb246106a37dbf3f3) Bug: 1039869 Change-Id: I17d3a1a5417a6cb3c261d388760a65127c38de4a Reviewed-by: Jüri Valdmann --- .../browser/appcache/appcache_manifest_parser.cc | 38 +++++----------------- .../content/browser/appcache/appcache_namespace.h | 5 ++- 2 files changed, 13 insertions(+), 30 deletions(-) diff --git a/chromium/content/browser/appcache/appcache_manifest_parser.cc b/chromium/content/browser/appcache/appcache_manifest_parser.cc index d07cabf28fb..5b5d3851e18 100644 --- a/chromium/content/browser/appcache/appcache_manifest_parser.cc +++ b/chromium/content/browser/appcache/appcache_manifest_parser.cc @@ -186,17 +186,6 @@ Mode ParseModeSettingLine(base::StringPiece line) { return Mode::kUnknown; } -// True if the next token in the manifest line is the pattern indicator flag. -// -// Pattern URLs are a non-standard feature. -bool NextTokenIsPatternMatchingFlag(base::StringPiece line) { - base::StringPiece is_pattern_token; - std::tie(is_pattern_token, line) = SplitLineToken(line); - - static constexpr base::StringPiece kPatternFlag("isPattern"); - return is_pattern_token == kPatternFlag; -} - // Parses a URL token in an AppCache manifest. // // The returned URL may not be valid, if the token does not represent a valid @@ -352,14 +341,9 @@ bool ParseManifest(const GURL& manifest_url, if (mode == Mode::kExplicit) { manifest.explicit_urls.insert(namespace_url.spec()); } else { - // Chrome supports URL patterns in manifests. This is not standardized. - // An URL record followed by the "isPattern" token is considered a - // pattern. - - // TODO(pwnall): Add a UMA metric to see if we can remove this feature. - bool is_pattern = NextTokenIsPatternMatchingFlag(line); - manifest.online_whitelist_namespaces.emplace_back(AppCacheNamespace( - APPCACHE_NETWORK_NAMESPACE, namespace_url, GURL(), is_pattern)); + manifest.online_whitelist_namespaces.emplace_back( + AppCacheNamespace(APPCACHE_NETWORK_NAMESPACE, namespace_url, GURL(), + /*is_pattern=*/false)); } continue; } @@ -396,10 +380,9 @@ bool ParseManifest(const GURL& manifest_url, if (manifest_url.GetOrigin() != target_url.GetOrigin()) continue; - // TODO(pwnall): Add a UMA metric to see if we can remove this feature. - bool is_pattern = NextTokenIsPatternMatchingFlag(line); - manifest.intercept_namespaces.emplace_back( - APPCACHE_INTERCEPT_NAMESPACE, namespace_url, target_url, is_pattern); + manifest.intercept_namespaces.emplace_back(APPCACHE_INTERCEPT_NAMESPACE, + namespace_url, target_url, + /*is_pattern=*/false); continue; } @@ -425,14 +408,11 @@ bool ParseManifest(const GURL& manifest_url, if (manifest_url.GetOrigin() != fallback_url.GetOrigin()) continue; - // TODO(pwnall): Add a UMA metric to see if we can remove this feature. - bool is_pattern = NextTokenIsPatternMatchingFlag(line); - // Store regardless of duplicate namespace URL. Only the first match will // ever be used. - manifest.fallback_namespaces.emplace_back( - AppCacheNamespace(APPCACHE_FALLBACK_NAMESPACE, namespace_url, - fallback_url, is_pattern)); + manifest.fallback_namespaces.emplace_back(APPCACHE_FALLBACK_NAMESPACE, + namespace_url, fallback_url, + /*is_pattern=*/false); continue; } diff --git a/chromium/content/browser/appcache/appcache_namespace.h b/chromium/content/browser/appcache/appcache_namespace.h index 709d5ce13aa..b08f9a4aed9 100644 --- a/chromium/content/browser/appcache/appcache_namespace.h +++ b/chromium/content/browser/appcache/appcache_namespace.h @@ -29,9 +29,12 @@ struct CONTENT_EXPORT AppCacheNamespace { AppCacheNamespaceType type; GURL namespace_url; GURL target_url; + + // TODO(jsbell): Remove pattern support, since it has been removed from the + // parser already. bool is_pattern; }; } // namespace content -#endif // CONTENT_BROWSER_APPCACHE_APPCACHE_NAMESPACE_H_ \ No newline at end of file +#endif // CONTENT_BROWSER_APPCACHE_APPCACHE_NAMESPACE_H_ -- cgit v1.2.1