From 6b9c19ecebdd172b0381c37d49443a1f3811176f Mon Sep 17 00:00:00 2001 From: Egon Okerman Date: Wed, 26 Mar 2025 11:57:39 +0100 Subject: [PATCH] Update rule S7201: Disable rule and move rule text to S6363 (SONARKT-636) (#4802) * Close S7201 * Update S6363 with updated descriptions * Update OWASP categories with S7201 info --- rules/S6363/ask-yourself.adoc | 6 ++-- rules/S6363/description.adoc | 12 +++---- rules/S6363/kotlin/rule.adoc | 60 ++++++++++++++++++++++++++------ rules/S6363/metadata.json | 8 ++--- rules/S6363/recommended.adoc | 9 ++--- rules/S6363/see.adoc | 6 ++-- rules/S7201/kotlin/metadata.json | 4 +-- rules/S7201/metadata.json | 3 +- 8 files changed, 76 insertions(+), 32 deletions(-) diff --git a/rules/S6363/ask-yourself.adoc b/rules/S6363/ask-yourself.adoc index b90cdc13ee..561f647513 100644 --- a/rules/S6363/ask-yourself.adoc +++ b/rules/S6363/ask-yourself.adoc @@ -1,6 +1,6 @@ == Ask Yourself Whether -* No local files have to be accessed by the Webview. -* The WebView contains untrusted data that could cause harm when rendered. +* You open files that may be created or altered by external sources. +* You open arbitrary URLs from external sources. -There is a risk if you answered yes to any of those questions. +There is a risk if you answered yes to any of these questions. diff --git a/rules/S6363/description.adoc b/rules/S6363/description.adoc index baafce45c7..a80257ec5e 100644 --- a/rules/S6363/description.adoc +++ b/rules/S6363/description.adoc @@ -1,7 +1,7 @@ -WebViews can be used to display web content as part of a mobile application. A -browser engine is used to render and display the content. Like a web -application, a mobile application that uses WebViews can be vulnerable to -Cross-Site Scripting if untrusted code is rendered. +Exposing the Android file system to WebViews is security-sensitive. -If malicious JavaScript code in a WebView is executed this can leak the contents -of sensitive files when access to local files is enabled. +Granting file access to WebViews, particularly through the `file://` scheme, introduces a risk of local file inclusion +vulnerabilities. The severity of this risk depends heavily on the specific `WebSettings` configured. Overly permissive +settings can allow malicious scripts to access a wide range of local files, potentially exposing sensitive data such as +Personally Identifiable Information (PII) or private application data, leading to data breaches and other security +compromises. diff --git a/rules/S6363/kotlin/rule.adoc b/rules/S6363/kotlin/rule.adoc index e408da2f3c..fa4b9b6919 100644 --- a/rules/S6363/kotlin/rule.adoc +++ b/rules/S6363/kotlin/rule.adoc @@ -8,26 +8,66 @@ include::../recommended.adoc[] [source,kotlin] ---- -import android.webkit.WebView - -val webView: WebView = findViewById(R.id.webview) -webView.getSettings().setAllowContentAccess(true) // Sensitive -webView.getSettings().setAllowFileAccess(true) // Sensitive +AndroidView( + factory = { context -> + WebView(context).apply { + webViewClient = WebViewClient() + settings.apply { + allowFileAccess = true // Sensitive + allowFileAccessFromFileURLs = true // Sensitive + allowUniversalAccessFromFileURLs = true // Sensitive + allowContentAccess = true // Sensitive + } + loadUrl("file:///android_asset/example.html") + } + } +) ---- == Compliant Solution [source,kotlin] ---- -import android.webkit.WebView +AndroidView( + factory = { context -> + val webView = WebView(context) + val assetLoader = WebViewAssetLoader.Builder() + .addPathHandler("/assets/", WebViewAssetLoader.AssetsPathHandler(context)) + .build() -val webView: WebView = findViewById(R.id.webview) -webView.getSettings().setAllowContentAccess(false) -webView.getSettings().setAllowFileAccess(false) + webView.webViewClient = object : WebViewClient() { + @RequiresApi(Build.VERSION_CODES.LOLLIPOP) + override fun shouldInterceptRequest(view: WebView?, request: WebResourceRequest): WebResourceResponse? { + return assetLoader.shouldInterceptRequest(request.url) + } + + @Suppress("deprecation") + override fun shouldInterceptRequest(view: WebView?, url: String?): WebResourceResponse? { + return assetLoader.shouldInterceptRequest(Uri.parse(url)) + } + } + + webView.settings.apply { + allowFileAccess = false + allowFileAccessFromFileURLs = false + allowUniversalAccessFromFileURLs = false + allowContentAccess = false + } + + webView.loadUrl("https://appassets.androidplatform.net/assets/example.html") + webView + } +) ---- -include::../see.adoc[] +The compliant solution uses `WebViewAssetLoader` to load local files instead of directly accessing them via `file://` +URLs. This approach serves assets over a secure `https://appassets.androidplatform.net` URL, effectively isolating the +WebView from the local file system. +The file access settings are disabled by default in modern Android versions. To prevent possible security issues in +`Build.VERSION_CODES.Q` and earlier, it is still recommended to explicitly set those values to false. + +include::../see.adoc[] ifdef::env-github,rspecator-view[] diff --git a/rules/S6363/metadata.json b/rules/S6363/metadata.json index 4f906b677f..9a93f356ff 100644 --- a/rules/S6363/metadata.json +++ b/rules/S6363/metadata.json @@ -26,8 +26,8 @@ 79 ], "OWASP": [ - "A6", - "A7" + "A3", + "A6" ], "MASVS": [ "MSTG-PLATFORM-2" @@ -36,7 +36,7 @@ "M8" ], "OWASP Top 10 2021": [ - "A3" + "A1" ], "PCI DSS 3.2": [ "6.5.1", @@ -53,4 +53,4 @@ "Sonar way" ], "quickfix": "unknown" -} +} \ No newline at end of file diff --git a/rules/S6363/recommended.adoc b/rules/S6363/recommended.adoc index 78d58cf95c..3173b73454 100644 --- a/rules/S6363/recommended.adoc +++ b/rules/S6363/recommended.adoc @@ -1,6 +1,7 @@ == Recommended Secure Coding Practices -It is recommended to disable access to local files for WebViews unless it is -necessary. In the case of a successful attack through a Cross-Site Scripting -vulnerability the attackers attack surface decreases drastically if no files -can be read out. +Avoid opening `file://` URLs from external sources in WebView components. If your application accepts arbitrary URLs +from external sources, do not enable this functionality. Instead, utilize `androidx.webkit.WebViewAssetLoader` to access +files, including assets and resources, via `http(s)://` schemes. + +For enhanced security, ensure that the options to load `file://` URLs are explicitly set to false. diff --git a/rules/S6363/see.adoc b/rules/S6363/see.adoc index fa0c2e2d66..ede042cddb 100644 --- a/rules/S6363/see.adoc +++ b/rules/S6363/see.adoc @@ -1,7 +1,9 @@ == See -* OWASP - https://owasp.org/Top10/A03_2021-Injection/[Top 10 2021 Category A3 - Injection] +* OWASP - https://owasp.org/Top10/A01_2021-Broken_Access_Control/[Top 10 2021 Category A1 - Broken Access Control] +* OWASP - https://owasp.org/www-project-top-ten/2017/A3_2017-Sensitive_Data_Exposure[Top 10 2017 Category A3 - Sensitive Data Exposure] * OWASP - https://owasp.org/www-project-top-ten/2017/A6_2017-Security_Misconfiguration[Top 10 2017 Category A6 - Security Misconfiguration] -* OWASP - https://owasp.org/www-project-top-ten/2017/A7_2017-Cross-Site_Scripting_(XSS)[Top 10 2017 Category A7 - Cross-Site Scripting (XSS)] * OWASP - https://owasp.org/www-project-mobile-top-10/2023-risks/m8-security-misconfiguration[Mobile Top 10 2024 Category M8 - Security Misconfiguration] +* OWASP - https://mas.owasp.org/checklists/MASVS-PLATFORM/[Mobile AppSec Verification Standard - Platform Interaction Requirements] * CWE - https://cwe.mitre.org/data/definitions/79[CWE-79 - Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')] +* Android Documentation - https://developer.android.com/privacy-and-security/risks/webview-unsafe-file-inclusion[WebViews - Unsafe File Inclusion] diff --git a/rules/S7201/kotlin/metadata.json b/rules/S7201/kotlin/metadata.json index a097fc7c65..cf0e52892e 100644 --- a/rules/S7201/kotlin/metadata.json +++ b/rules/S7201/kotlin/metadata.json @@ -7,7 +7,7 @@ }, "attribute": "COMPLETE" }, - "status": "ready", + "status": "closed", "remediation": { "func": "Constant\/Issue", "constantCost": "30min" @@ -43,4 +43,4 @@ "defaultQualityProfiles": [ "Sonar way" ] -} +} \ No newline at end of file diff --git a/rules/S7201/metadata.json b/rules/S7201/metadata.json index 2c63c08510..c675b4bb09 100644 --- a/rules/S7201/metadata.json +++ b/rules/S7201/metadata.json @@ -1,2 +1,3 @@ { -} + "status": "closed" +} \ No newline at end of file