summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAllan Sandfeld Jensen <allan.jensen@qt.io>2021-09-23 16:51:21 +0200
committerAllan Sandfeld Jensen <allan.jensen@qt.io>2021-10-12 09:07:55 +0200
commitf6f8f258be09fef90585b0228bd82a9708ef34a6 (patch)
treebda96d216ba4c287fc516fa3345ae27feaaac437
parent9a6589369a0c6bfff31ef644f70bb54fd22ba17c (diff)
Block CORS from local URLs when remote access is not enabled
Pick-to: 6.2 5.15 Task-number: QTBUG-96849 Change-Id: I0e0a1530b8b31341c632a1fd00abd339b5152da0 Reviewed-by: Kirill Burtsev <kirill.burtsev@qt.io>
-rw-r--r--src/core/doc/src/qwebenginesettings_lgpl.qdoc13
-rw-r--r--src/core/net/proxying_url_loader_factory_qt.cpp37
-rw-r--r--tests/auto/core/origins/tst_origins.cpp2
3 files changed, 44 insertions, 8 deletions
diff --git a/src/core/doc/src/qwebenginesettings_lgpl.qdoc b/src/core/doc/src/qwebenginesettings_lgpl.qdoc
index 7c6ccd3d3..bfe2713a2 100644
--- a/src/core/doc/src/qwebenginesettings_lgpl.qdoc
+++ b/src/core/doc/src/qwebenginesettings_lgpl.qdoc
@@ -97,13 +97,11 @@
Enables support for the HTML 5 local storage feature. Enabled by default.
\value LocalContentCanAccessRemoteUrls
Allows locally loaded documents to ignore cross-origin rules so that they can access
- remote resources that would normally be blocked, because all remote resources are
- considered cross-origin for a local file. Remote access that would not be blocked by
+ remote resources that would normally be blocked, since remote resources are
+ considered cross-origin for a local document. Remote access that would not be blocked by
cross-origin rules is still possible when this setting is disabled (default).
- Note that disabling this setting does not stop XMLHttpRequests or media elements in
- local files from accessing remote content. Basically, it only stops some HTML
- subresources, such as scripts, and therefore disabling this setting is not a safety
- mechanism.
+ Note that disabling this setting does not prevent media elements in local files from
+ accessing remote content. Disabled by default.
\value XSSAuditingEnabled
Obsolete and has no effect.
\value SpatialNavigationEnabled
@@ -114,7 +112,8 @@
trying to reach towards the right and which element they probably want.
Disabled by default.
\value LocalContentCanAccessFileUrls
- Allows locally loaded documents to access other local URLs. Enabled by default.
+ Allows locally loaded documents to access other local URLs. Disabling this makes QtWebEngine
+ behave more like Chrome and Firefox does by default. Enabled by default.
\value HyperlinkAuditingEnabled
Enables support for the \c ping attribute for hyperlinks. Disabled by default.
\value ScrollAnimatorEnabled
diff --git a/src/core/net/proxying_url_loader_factory_qt.cpp b/src/core/net/proxying_url_loader_factory_qt.cpp
index c73da1455..6505e5505 100644
--- a/src/core/net/proxying_url_loader_factory_qt.cpp
+++ b/src/core/net/proxying_url_loader_factory_qt.cpp
@@ -47,8 +47,11 @@
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/web_contents.h"
+#include "content/public/common/content_switches.h"
#include "net/http/http_status_code.h"
+#include "services/network/public/cpp/cors/cors.h"
#include "third_party/blink/public/mojom/loader/resource_load_info.mojom-shared.h"
+#include "url/url_util.h"
#include "api/qwebengineurlrequestinfo_p.h"
#include "type_conversion.h"
@@ -164,6 +167,7 @@ private:
const uint64_t request_id_;
const int32_t routing_id_;
const uint32_t options_;
+ bool allowed_cors_ = true;
// If the |target_loader_| called OnComplete with an error this stores it.
// That way the destructor can send it to OnReceivedError if safe browsing
@@ -205,12 +209,37 @@ InterceptedRequest::InterceptedRequest(ProfileAdapter *profile_adapter,
, target_factory_(std::move(target_factory))
, weak_factory_(this)
{
+ const bool disable_web_security = base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kDisableWebSecurity);
current_response_ = network::mojom::URLResponseHead::New();
+ current_response_->response_type = network::cors::CalculateResponseType(
+ request_.mode,
+ disable_web_security || (
+ request_.request_initiator && request_.request_initiator->IsSameOriginWith(url::Origin::Create(request_.url))));
// If there is a client error, clean up the request.
target_client_.set_disconnect_handler(
base::BindOnce(&InterceptedRequest::OnURLLoaderClientError, base::Unretained(this)));
proxied_loader_receiver_.set_disconnect_with_reason_handler(
base::BindOnce(&InterceptedRequest::OnURLLoaderError, base::Unretained(this)));
+ if (!disable_web_security && request_.request_initiator) {
+ const std::vector<std::string> &localSchemes = url::GetLocalSchemes();
+ std::string fromScheme = request_.request_initiator->GetTupleOrPrecursorTupleIfOpaque().scheme();
+ if (base::Contains(localSchemes, fromScheme)) {
+ content::WebContents *wc = webContents();
+ std::string toScheme = request_.url.scheme();
+ // local schemes must have universal access, or be accessing something local and have local access.
+ if (fromScheme != toScheme) {
+ // note allow_file_access_from_file_urls maps to LocalContentCanAccessFileUrls in our API
+ // and allow_universal_access_from_file_urls to LocalContentCanAccessRemoteUrls, so we are
+ // using them as proxies for our API here.
+ if (toScheme == "file")
+ allowed_cors_ = wc && wc->GetOrCreateWebPreferences().allow_file_access_from_file_urls;
+ else if (!base::Contains(localSchemes, toScheme))
+ allowed_cors_ = wc && wc->GetOrCreateWebPreferences().allow_universal_access_from_file_urls;
+ else
+ allowed_cors_ = true; // We should think about this for future patches
+ }
+ }
+ }
}
InterceptedRequest::~InterceptedRequest()
@@ -247,6 +276,14 @@ void InterceptedRequest::Restart()
{
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+ // This is a CORS check on the from URL, the normal check on the to URL is applied later
+ if (!allowed_cors_ && current_response_->response_type == network::mojom::FetchResponseType::kCors) {
+ target_client_->OnComplete(network::URLLoaderCompletionStatus(
+ network::CorsErrorStatus(network::mojom::CorsError::kCorsDisabledScheme)));
+ delete this;
+ return;
+ }
+
// MEMO since all codepatch leading to Restart scheduled and executed as asynchronous tasks in main thread,
// interceptors may change in meantime and also during intercept call, so they should be resolved anew.
// Set here only profile's interceptor since it runs first without going to user code.
diff --git a/tests/auto/core/origins/tst_origins.cpp b/tests/auto/core/origins/tst_origins.cpp
index 518b20045..a8ec5b7b0 100644
--- a/tests/auto/core/origins/tst_origins.cpp
+++ b/tests/auto/core/origins/tst_origins.cpp
@@ -672,7 +672,7 @@ void tst_Origins::mixedXHR_data()
QTest::newRow("file->cors") << QString("file:" + QDir(QT_TESTCASE_SOURCEDIR).canonicalPath()
+ "/resources/mixedXHR.html")
<< QString("sendXHR('cors:/resources/mixedXHR.txt')")
- << QVariant(QString("ok"));
+ << QVariant(QString("error"));
QTest::newRow("qrc->file") << QString("qrc:/resources/mixedXHR.html")
<< QString("sendXHR('file:"