Improve local scheme access rules

Pick-to: 6.3 6.2
Task-number: QTBUG-96849
Change-Id: Ieb24da12a61e5e37b29ccf2d1a11b7bd863b842e
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

6 files changed, 57 insertions, 43 deletions

diff --git a/src/core/api/qwebengineurlscheme.cpp b/src/core/api/qwebengineurlscheme.cpp
index f73992c6b..c187fc840 100644
--- a/src/core/api/qwebengineurlscheme.cpp
+++ b/src/core/api/qwebengineurlscheme.cpp
@@ -171,12 +171,12 @@ public:
   Indicates that the URL scheme provides access to local resources. The purpose
   of this flag is to prevent network content from accessing local resources.
   Only schemes with the \c LocalAccessAllowed flag may load resources from a
-  scheme with the \c Local flag. The only builtin schemes with this flag are \c
-  file and \c qrc.
+  scheme with the \c LocalScheme flag. The only builtin scheme with this flag is \c
+  file.
 
   \value LocalAccessAllowed
   Indicates that content from this scheme should be allowed to load resources
-  from schemes with the \c Local flag.
+  from schemes with the \c LocalScheme flag.
 
   \value NoAccessAllowed
   Indicates that all content from this scheme should be forced to have unique
@@ -194,10 +194,10 @@ public:
 
   \value CorsEnabled
   Enables cross-origin resource sharing (CORS) for this scheme. This flag is
-  required in order to, for example, use the scheme with the \l
-  {https://fetch.spec.whatwg.org/}{Fetch API}, or to deliver CSS fonts to a
-  different origin. The appropriate CORS headers are generated automatically by
-  the QWebEngineUrlRequestJob class. (Added in Qt 5.14)
+  required in order for content to be loaded by documents of a different origin,
+  this includes access from other schemes. The appropriate CORS headers are
+  generated automatically by the QWebEngineUrlRequestJob class. By default only
+  \c http and \c https are CORS enabled. (Added in Qt 5.14)
 */
 
 QWebEngineUrlScheme::QWebEngineUrlScheme(QWebEngineUrlSchemePrivate *d) : d(d) {}
diff --git a/src/core/content_browser_client_qt.cpp b/src/core/content_browser_client_qt.cpp
index 2b1485b2d..6201bab27 100644
--- a/src/core/content_browser_client_qt.cpp
+++ b/src/core/content_browser_client_qt.cpp
@@ -1102,15 +1102,10 @@ void ContentBrowserClientQt::RegisterNonNetworkSubresourceURLLoaderFactories(int
     // Install file scheme if necessary:
     bool install_file_scheme = false;
     if (web_contents && !is_background_page) {
-        const auto *settings = static_cast<WebContentsDelegateQt *>(web_contents->GetResponsibleWebContents()->GetDelegate())->webEngineSettings();
-        if (settings->testAttribute(QWebEngineSettings::LocalContentCanAccessFileUrls)) {
-            for (const auto &local_scheme : url::GetLocalSchemes()) {
-                if (url.SchemeIs(local_scheme)) {
-                    install_file_scheme = true;
-                    break;
-                }
-            }
-        }
+        const std::string scheme = url.scheme();
+        install_file_scheme = base::Contains(url::GetLocalSchemes(), scheme);
+        if (const url::CustomScheme *cs = url::CustomScheme::FindScheme(scheme))
+            install_file_scheme = cs->flags & (url::CustomScheme::LocalAccessAllowed | url::CustomScheme::Local);
     }
 
     if (install_file_scheme && factories->find(url::kFileScheme) == factories->end()) {
diff --git a/src/core/doc/src/qwebenginesettings_lgpl.qdoc b/src/core/doc/src/qwebenginesettings_lgpl.qdoc
index bfe2713a2..a08b13f2c 100644
--- a/src/core/doc/src/qwebenginesettings_lgpl.qdoc
+++ b/src/core/doc/src/qwebenginesettings_lgpl.qdoc
@@ -96,10 +96,7 @@
     \value  LocalStorageEnabled
             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, 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).
+            Allows local origin documents to access remote resources that would normally be blocked.
             Note that disabling this setting does not prevent media elements in local files from
             accessing remote content. Disabled by default.
     \value  XSSAuditingEnabled
diff --git a/src/core/net/custom_url_loader_factory.cpp b/src/core/net/custom_url_loader_factory.cpp
index 283576cac..6cb272b1c 100644
--- a/src/core/net/custom_url_loader_factory.cpp
+++ b/src/core/net/custom_url_loader_factory.cpp
@@ -55,6 +55,8 @@
 #include "services/network/public/mojom/url_loader.mojom.h"
 #include "services/network/public/mojom/url_loader_factory.mojom.h"
 #include "services/network/public/mojom/url_response_head.mojom.h"
+#include "url/url_util.h"
+#include "url/url_util_qt.h"
 
 #include "api/qwebengineurlscheme.h"
 #include "net/url_request_custom_job_proxy.h"
@@ -135,6 +137,7 @@ private:
         m_error = 0;
         QWebEngineUrlScheme scheme = QWebEngineUrlScheme::schemeByName(QByteArray::fromStdString(request.url.scheme()));
         m_corsEnabled = scheme.flags().testFlag(QWebEngineUrlScheme::CorsEnabled);
+        m_isLocal = scheme.flags().testFlag(QWebEngineUrlScheme::LocalScheme);
     }
 
     ~CustomURLLoader() override = default;
@@ -148,10 +151,21 @@ private:
             if (!m_request.request_initiator)
                 return CompleteWithFailure(net::ERR_INVALID_ARGUMENT);
 
-            // Custom schemes are not covered by CorsURLLoader, so we need to reject CORS requests manually.
-            if (!m_corsEnabled && !m_request.request_initiator->IsSameOriginWith(url::Origin::Create(m_request.url)))
-                return CompleteWithFailure(network::CorsErrorStatus(network::mojom::CorsError::kCorsDisabledScheme));
+            if (m_isLocal) {
+                std::string fromScheme = m_request.request_initiator->GetTupleOrPrecursorTupleIfOpaque().scheme();
+                const std::vector<std::string> &localSchemes = url::GetLocalSchemes();
+                bool fromLocal = base::Contains(localSchemes, fromScheme);
+                bool hasLocalAccess = fromLocal;
+                if (const url::CustomScheme *cs = url::CustomScheme::FindScheme(fromScheme))
+                    hasLocalAccess = cs->flags & (url::CustomScheme::LocalAccessAllowed | url::CustomScheme::Local);
+                if (!hasLocalAccess)
+                    return CompleteWithFailure(net::ERR_ACCESS_DENIED);
+            } else if (!m_corsEnabled && !m_request.request_initiator->IsSameOriginWith(url::Origin::Create(m_request.url))) {
+                // Custom schemes are not covered by CorsURLLoader, so we need to reject CORS requests manually.
+                 return CompleteWithFailure(network::CorsErrorStatus(network::mojom::CorsError::kCorsDisabledScheme));
+            }
         }
+
         if (mojo::CreateDataPipe(nullptr, m_pipeProducerHandle, m_pipeConsumerHandle) != MOJO_RESULT_OK)
             return CompleteWithFailure(net::ERR_FAILED);
 
@@ -453,6 +467,7 @@ private:
     qint64 m_headerBytesRead = 0;
     qint64 m_totalBytesRead = 0;
     bool m_corsEnabled;
+    bool m_isLocal;
 
     base::WeakPtrFactory<CustomURLLoader> m_weakPtrFactory{this};
 
diff --git a/src/core/net/proxying_url_loader_factory_qt.cpp b/src/core/net/proxying_url_loader_factory_qt.cpp
index be2a6814b..9bdebf6be 100644
--- a/src/core/net/proxying_url_loader_factory_qt.cpp
+++ b/src/core/net/proxying_url_loader_factory_qt.cpp
@@ -53,6 +53,7 @@
 #include "services/network/public/mojom/early_hints.mojom.h"
 #include "third_party/blink/public/mojom/loader/resource_load_info.mojom-shared.h"
 #include "url/url_util.h"
+#include "url/url_util_qt.h"
 
 #include "api/qwebengineurlrequestinfo_p.h"
 #include "type_conversion.h"
@@ -168,7 +169,10 @@ private:
     const int frame_tree_node_id_;
     const int32_t request_id_;
     const uint32_t options_;
-    bool allowed_cors_ = true;
+    bool allow_local_ = false;
+    bool allow_remote_ = true;
+    bool local_access_ = false;
+    bool remote_access_ = 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
@@ -222,22 +226,20 @@ InterceptedRequest::InterceptedRequest(ProfileAdapter *profile_adapter,
             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)) {
+        const std::string fromScheme = request_.request_initiator->GetTupleOrPrecursorTupleIfOpaque().scheme();
+        const std::string toScheme = request_.url.scheme();
+        const bool fromLocal = base::Contains(localSchemes, fromScheme);
+        const bool toLocal = base::Contains(localSchemes, toScheme);
+        bool hasLocalAccess = false;
+        local_access_ = toLocal;
+        remote_access_ = !toLocal && (toScheme != "data") && (toScheme != "qrc");
+        if (const url::CustomScheme *cs = url::CustomScheme::FindScheme(fromScheme))
+            hasLocalAccess = cs->flags & url::CustomScheme::LocalAccessAllowed;
+        if (fromLocal || toLocal) {
             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
-            }
+            allow_local_ = hasLocalAccess || (fromLocal && wc && wc->GetOrCreateWebPreferences().allow_file_access_from_file_urls);
+            allow_remote_ = !fromLocal || (wc && wc->GetOrCreateWebPreferences().allow_remote_access_from_local_urls);
         }
     }
 }
@@ -273,10 +275,15 @@ 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)));
+    // Check if non-local access is allowed
+    if (!allow_remote_ && remote_access_) {
+        target_client_->OnComplete(network::URLLoaderCompletionStatus(net::ERR_NETWORK_ACCESS_DENIED));
+        delete this;
+        return;
+    }
+    // Check if local access is allowed
+    if (!allow_local_ && local_access_) {
+        target_client_->OnComplete(network::URLLoaderCompletionStatus(net::ERR_ACCESS_DENIED));
         delete this;
         return;
     }
diff --git a/src/core/web_engine_settings.cpp b/src/core/web_engine_settings.cpp
index 007249f42..a04efdd1f 100644
--- a/src/core/web_engine_settings.cpp
+++ b/src/core/web_engine_settings.cpp
@@ -377,7 +377,7 @@ void WebEngineSettings::applySettingsToWebPreferences(blink::web_pref::WebPrefer
     prefs->tabs_to_links = testAttribute(QWebEngineSettings::LinksIncludedInFocusChain);
     prefs->local_storage_enabled = testAttribute(QWebEngineSettings::LocalStorageEnabled);
     prefs->databases_enabled = testAttribute(QWebEngineSettings::LocalStorageEnabled);
-    prefs->allow_universal_access_from_file_urls =
+    prefs->allow_remote_access_from_local_urls =
             testAttribute(QWebEngineSettings::LocalContentCanAccessRemoteUrls);
     prefs->spatial_navigation_enabled = testAttribute(QWebEngineSettings::SpatialNavigationEnabled);
     prefs->allow_file_access_from_file_urls =