Improve local scheme access rules

Task-number: QTBUG-96849
Change-Id: Ieb24da12a61e5e37b29ccf2d1a11b7bd863b842e
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
(cherry picked from commit 3071de1e07be28d763164a037d946281146bf31d)
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

2 files changed, 44 insertions, 22 deletions

diff --git a/src/core/net/custom_url_loader_factory.cpp b/src/core/net/custom_url_loader_factory.cpp
index dca69c20f..b237394f2 100644
--- a/src/core/net/custom_url_loader_factory.cpp
+++ b/src/core/net/custom_url_loader_factory.cpp
@@ -54,6 +54,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"
@@ -134,6 +136,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;
@@ -147,10 +150,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);
 
@@ -452,6 +466,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 6505e5505..117ace394 100644
--- a/src/core/net/proxying_url_loader_factory_qt.cpp
+++ b/src/core/net/proxying_url_loader_factory_qt.cpp
@@ -52,6 +52,7 @@
 #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 "url/url_util_qt.h"
 
 #include "api/qwebengineurlrequestinfo_p.h"
 #include "type_conversion.h"
@@ -167,7 +168,10 @@ private:
     const uint64_t request_id_;
     const int32_t routing_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);
         }
     }
 }
@@ -276,10 +278,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;
     }