7b07879d6e
Motivation for this CL ====================== Before this CL url_formatter::FixupURL could transform URLs of renderer-initiated navigations. This may have allowed an attacker to "launder" URLs past early filters (if the initial URL provided by the attacker looks benign, but after passing the filters is transformed by FixupURL into a malicious form). The following bugs in the past seem to have been at least partially enabled by this transformation: - Laundering a "javascript:" URL: https://crbug.com/1116280 and https://crbug.com/850824 - Omnibox spoof due to the modified url: https://crbug.com/449829 and https://crbug.com/657720 - Invariant violation due to a difference between virtual url and regular url: https://crbug.com/895065 Before this CL, url_formatter::FixupURL was called for renderer-initiated navigations via WillHandleBrowserAboutURL. This doesn’t seem necessary per the TODO in this function asking to "Eliminate "about:*" constants [...] then hopefully we can remove this forced fixup". Note that BrowserURLHandler::SetFixupHandler has been introduced in r316923 as a workaround to ensure URL consistency by adding FixupURL in one more place. If FixupURL wouldn’t be called from WillHandleBrowserAboutURL, then BrowserURLHandler::SetFixupHandler wouldn’t be needed in the first place. Finally, https://crbug.com/1130091#c3 points out that url_formatter::FixupURL should only be invoked on user input (e.g. URLs typed into the omnibox and used for browser-initiated navigations) rather than on arbitrary URLs. Summary of changes in this CL ============================= Based on the above, the CL: - Removes the FixupBrowserAboutURL call from WillHandleBrowserAboutURL - Removes the BrowserURLHandler::SetFixupHandler method and related code - Fixes tests so that they use the final URL form (rather than relying on the FixupURL call) - Adjusting expectations of tests that were testing laundering scenarios The CL also opportunistically: - Simplifies WillHandleBrowserAboutURL (path transformations were a no-op; early return for kChromeUISettingsHost and kChromeUIHelpHost can be consolidated) Testing ======= Manual testing: - Verified that "about:version" typed into the omnibox still ends up navigating to "chrome://version/" - Verified that renderer-initiated navigations to "about:version" are translated into "about:blank#blocked" by RenderProcessHostImpl::FilterURL Bug: 1130091 Change-Id: I9a9f32d4d9c0ec630c2110679efe0c1d18b4370b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2441284 Reviewed-by: Charlie Reis <creis@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org> Cr-Commit-Position: refs/heads/master@{#818969}
69 lines
2.4 KiB
C++
69 lines
2.4 KiB
C++
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
|
|
// Use of this source code is governed by a BSD-style license that can be
|
|
// found in the LICENSE file.
|
|
|
|
#ifndef CONTENT_BROWSER_BROWSER_URL_HANDLER_IMPL_H_
|
|
#define CONTENT_BROWSER_BROWSER_URL_HANDLER_IMPL_H_
|
|
|
|
#include <utility>
|
|
#include <vector>
|
|
|
|
#include "base/gtest_prod_util.h"
|
|
#include "base/macros.h"
|
|
#include "base/memory/singleton.h"
|
|
#include "content/public/browser/browser_url_handler.h"
|
|
|
|
class GURL;
|
|
|
|
namespace content {
|
|
class BrowserContext;
|
|
|
|
class CONTENT_EXPORT BrowserURLHandlerImpl : public BrowserURLHandler {
|
|
public:
|
|
// Returns the singleton instance.
|
|
static BrowserURLHandlerImpl* GetInstance();
|
|
|
|
// BrowserURLHandler implementation:
|
|
void RewriteURLIfNecessary(GURL* url,
|
|
BrowserContext* browser_context) override;
|
|
std::vector<GURL> GetPossibleRewrites(
|
|
const GURL& url,
|
|
BrowserContext* browser_context) override;
|
|
void AddHandlerPair(URLHandler handler, URLHandler reverse_handler) override;
|
|
|
|
// Like the //content-public RewriteURLIfNecessary overload (overridden
|
|
// above), but if the original URL needs to be adjusted if the modified URL is
|
|
// redirected, this method sets |*reverse_on_redirect| to true.
|
|
void RewriteURLIfNecessary(GURL* url,
|
|
BrowserContext* browser_context,
|
|
bool* reverse_on_redirect);
|
|
|
|
// Reverses the rewriting that was done for |original| using the new |url|.
|
|
bool ReverseURLRewrite(GURL* url, const GURL& original,
|
|
BrowserContext* browser_context);
|
|
|
|
// Reverses |AddHandlerPair| for the given |handler|.
|
|
void RemoveHandlerForTesting(URLHandler handler);
|
|
|
|
private:
|
|
// This object is a singleton:
|
|
BrowserURLHandlerImpl();
|
|
~BrowserURLHandlerImpl() override;
|
|
friend struct base::DefaultSingletonTraits<BrowserURLHandlerImpl>;
|
|
|
|
// The list of known URLHandlers, optionally with reverse-rewriters.
|
|
typedef std::pair<URLHandler, URLHandler> HandlerPair;
|
|
std::vector<HandlerPair> url_handlers_;
|
|
|
|
FRIEND_TEST_ALL_PREFIXES(BrowserURLHandlerImplTest, BasicRewriteAndReverse);
|
|
FRIEND_TEST_ALL_PREFIXES(BrowserURLHandlerImplTest, NullHandlerReverse);
|
|
FRIEND_TEST_ALL_PREFIXES(BrowserURLHandlerImplTest, ViewSourceReverse);
|
|
FRIEND_TEST_ALL_PREFIXES(BrowserURLHandlerImplTest, GetPossibleRewrites);
|
|
|
|
DISALLOW_COPY_AND_ASSIGN(BrowserURLHandlerImpl);
|
|
};
|
|
|
|
} // namespace content
|
|
|
|
#endif // CONTENT_BROWSER_BROWSER_URL_HANDLER_IMPL_H_
|