Compare commits

..

3 Commits

Author SHA1 Message Date
Quentin Jaquier
960696fb96 Fix from review: fix spelling 2025-03-28 11:33:19 +01:00
Quentin Jaquier
6bdb560e7c Add Kotlin rule description, minor java description change 2025-03-27 11:46:42 +01:00
quentin-jaquier-sonarsource
842cfdf9f9 Add kotlin to rule S6549 2025-03-27 11:46:42 +01:00
12 changed files with 120 additions and 128 deletions

View File

@ -5191,7 +5191,6 @@
"S1940": "sonar-kotlin 2.0.0.29",
"S2053": "sonar-kotlin 2.3.0.609",
"S2068": "sonar-kotlin 2.0.0.29",
"S2083": "sonar-security master",
"S2097": "sonar-kotlin 2.12.0.1956",
"S2114": "sonar-kotlin 2.12.0.1956",
"S2116": "sonar-kotlin 2.12.0.1956",
@ -5223,7 +5222,6 @@
"S5322": "sonar-kotlin 2.3.0.609",
"S5324": "sonar-kotlin 2.2.0.499",
"S5332": "sonar-kotlin 2.0.0.29",
"S5344": "sonar-kotlin master",
"S5527": "sonar-kotlin 2.0.0.29",
"S5542": "sonar-kotlin 2.0.0.29",
"S5547": "sonar-kotlin 2.0.0.29",
@ -5237,7 +5235,6 @@
"S5867": "sonar-kotlin 2.6.0.862",
"S5868": "sonar-kotlin 2.6.0.862",
"S5869": "sonar-kotlin 2.6.0.862",
"S6096": "sonar-security master",
"S6202": "sonar-kotlin 2.4.0.703",
"S6207": "sonar-kotlin 2.15.0.2579",
"S6218": "sonar-kotlin 2.4.0.703",
@ -5260,7 +5257,6 @@
"S6318": "sonar-kotlin 2.1.0.344",
"S6362": "sonar-kotlin 2.5.0.754",
"S6363": "sonar-kotlin 2.5.0.754",
"S6384": "sonar-security master",
"S6432": "sonar-kotlin 2.11.0.1828",
"S6474": "sonar-kotlin master",
"S6508": "sonar-kotlin 2.14.0.2352",

View File

@ -1,4 +1,4 @@
== How to fix it in Java I/O API
== How to fix it in Java SE
=== Code examples

View File

@ -4,7 +4,7 @@ include::../rationale.adoc[]
include::../impact.adoc[]
include::how-to-fix-it/java-io-api.adoc[]
include::how-to-fix-it/java-se.adoc[]
== Resources

View File

@ -1,90 +0,0 @@
== How to fix it in Java I/O API
=== Code examples
:canonicalization_function1: java.io.File.getCanonicalFile
:canonicalization_function2: java.io.File.getCanonicalPath
include::../../common/fix/code-rationale.adoc[]
==== Noncompliant code example
[source,kotlin,diff-id=1,diff-type=noncompliant]
----
class Example {
companion object {
private const val TARGET_DIRECTORY = "/example/directory/"
}
fun extractEntry(zipFile: ZipFile) {
val entries = zipFile.entries()
val entry = entries.nextElement()
val inputStream = zipFile.getInputStream(entry)
val file = File(TARGET_DIRECTORY + entry.name)
inputStream.copyTo(file.outputStream())
}
}
----
==== Compliant solution
[source,kotlin,diff-id=1,diff-type=compliant]
----
class Example {
companion object {
private const val TARGET_DIRECTORY = "/example/directory/"
}
fun extractEntry(zipFile: ZipFile) {
val entries = zipFile.entries()
val entry = entries.nextElement()
val inputStream = zipFile.getInputStream(entry)
val file = File(TARGET_DIRECTORY + entry.name)
val canonicalDestinationPath = file.canonicalPath
if (canonicalDestinationPath.startsWith(TARGET_DIRECTORY)) {
inputStream.copyTo(file.outputStream())
}
}
}
----
=== How does this work?
include::../../common/fix/how-does-this-work.adoc[]
=== Pitfalls
include::../../common/pitfalls/partial-path-traversal.adoc[]
For example, the following code is vulnerable to partial path injection. Note
that the string `targetDirectory` does not end with a path separator:
[source, kotlin]
----
companion object {
private const val targetDirectory = "/Users/John"
}
fun ExtractEntry(zipFile: ZipFile) {
val entries = zipFile.entries()
val entry = entries.nextElement()
val inputStream = zipFile.getInputStream(entry)
val file = File(entry.name)
val canonicalDestinationPath = file.canonicalPath
if (canonicalDestinationPath.startsWith(targetDirectory)) {
Files.copy(inputStream, file.toPath(), StandardCopyOption.REPLACE_EXISTING, LinkOption.NOFOLLOW_LINKS)
}
}
----
This check can be bypassed because `"/Users/Johnny".startsWith("/Users/John")`
returns `true`. Thus, for validation, `"/Users/John"` should actually be
`"/Users/John/"`.
**Warning**: Some functions, such as `.getCanonicalPath`, remove the
terminating path separator in their return value. +
The validation code should be tested to ensure that it cannot be impacted by this
issue.
https://github.com/aws/aws-sdk-java/security/advisories/GHSA-c28r-hw5m-5gv3[Here is a real-life example of this vulnerability.]

View File

@ -5,6 +5,3 @@
* 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]
* CWE - https://cwe.mitre.org/data/definitions/79[CWE-79 - Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')]
=== Related rules
* S7409 - Exposing Java objects through JavaScript interfaces is security-sensitive

View File

@ -1,4 +1,4 @@
== How to fix it in Java SE
== How to fix it in Java I/O API
=== Code examples
@ -16,11 +16,11 @@ public class ExampleController
static private String targetDirectory = "/path/to/target/directory/";
@GetMapping(value = "/exists")
public void delete(@RequestParam("filename") String filename) throws IOException {
public void exists(@RequestParam("filename") String filename) throws IOException {
File file = new File(targetDirectory + filename);
if (!file.exists()) { // Noncompliant
throw new IOException("File does not exists in the target directory");
throw new IOException("File does not exist in the target directory");
}
}
}
@ -38,7 +38,7 @@ public class ExampleController
static private String targetDirectory = "/path/to/target/directory/";
@GetMapping(value = "/exists")
public void delete(@RequestParam("filename") String filename) throws IOException {
public void exists(@RequestParam("filename") String filename) throws IOException {
File file = new File(targetDirectory + filename);
String canonicalDestinationPath = file.getCanonicalPath();
@ -46,7 +46,7 @@ public class ExampleController
if (!canonicalDestinationPath.startsWith(targetDirectory)) {
throw new IOException("Entry is outside of the target directory");
} else if (!file.exists()) {
throw new IOException("File does not exists in the target directory");
throw new IOException("File does not exist in the target directory");
}
}
}
@ -71,7 +71,7 @@ that the string `targetDirectory` does not end with a path separator:
static private String targetDirectory = "/Users/John";
@GetMapping(value = "/endpoint")
public void endpoint(@RequestParam("folder") fileName) throws IOException {
public void endpoint(@RequestParam("folder") File fileName) throws IOException {
String canonicalizedFileName = fileName.getCanonicalPath();

View File

@ -6,7 +6,7 @@ include::../impact.adoc[]
// How to fix it section
include::how-to-fix-it/java-se.adoc[]
include::how-to-fix-it/java-io-api.adoc[]
== Resources

View File

@ -0,0 +1,94 @@
== How to fix it in Java I/O API
=== Code examples
include::../../common/fix/code-rationale.adoc[]
==== Noncompliant code example
[source,kotlin,diff-id=1,diff-type=noncompliant]
----
@Controller
class ExampleController {
companion object {
private const val TARGET_DIRECTORY = "/path/to/target/directory/"
}
@GetMapping("/exists")
fun exists(@RequestParam("filename") filename: String) {
val file = File(TARGET_DIRECTORY + filename)
if (!file.exists()) { // Noncompliant
throw IOException("File does not exist in the target directory")
}
}
}
----
==== Compliant solution
[source,kotlin,diff-id=1,diff-type=compliant]
----
@Controller
class ExampleController {
companion object {
private const val TARGET_DIRECTORY = "/path/to/target/directory/"
}
@GetMapping("/exists")
fun exists(@RequestParam("filename") filename: String) {
val file = File(TARGET_DIRECTORY + filename)
val canonicalDestinationPath = file.getCanonicalPath()
if (!canonicalDestinationPath.startsWith(TARGET_DIRECTORY)) {
throw IOException("Entry is outside of the target directory")
} else if (!file.exists()) {
throw IOException("File does not exist in the target directory")
}
}
}
----
=== How does this work?
:canonicalization_function: java.io.File.getCanonicalPath
include::../../common/fix/canonical-path-validation.adoc[]
=== Pitfalls
include::../../common/pitfalls/partial-path-traversal.adoc[]
For example, the following code is vulnerable to partial path injection. Note
that the string `targetDirectory` does not end with a path separator:
[source, kotlin]
----
companion object {
private val TARGET_DIRECTORY = "/Users/John"
}
@GetMapping("/endpoint")
fun endpoint(@RequestParam("folder") file: File) {
val canonicalizedFileName = file.getCanonicalPath()
if (!canonicalizedFileName.startsWith(TARGET_DIRECTORY)) {
throw IOException("Entry is outside of the target directory")
}
}
----
This check can be bypassed if other directories start with `John`. For instance, `"/Users/Johnny".startsWith("/Users/John")`
returns `true`. Thus, for validation, `"/Users/John"` should actually be
`"/Users/John/"`.
**Warning**: Some functions, such as `getCanonicalPath`, remove the
terminating path separator in their return value. +
The validation code should be tested to ensure that it cannot be impacted by this
issue.
https://github.com/aws/aws-sdk-java/security/advisories/GHSA-c28r-hw5m-5gv3[Here is a real-life example of this vulnerability.]
:joining_docs: https://docs.oracle.com/javase/8/docs/api/java/nio/file/Path.html
:joining_func: java.nio.file.Path.resolve
include::../../common/pitfalls/oob-specific-path-joining.adoc[]

View File

@ -16,14 +16,13 @@
"M4"
],
"PCI DSS 3.2": [
"6.5.1",
"6.5.8"
],
"PCI DSS 4.0": [
"6.2.4"
],
"ASVS 4.0": [
"12.3.4",
"12.3.1",
"5.1.3",
"5.1.4"
],

View File

@ -4,12 +4,12 @@ include::../rationale.adoc[]
include::../impact.adoc[]
// How to fix it section
include::how-to-fix-it/java-io-api.adoc[]
== Resources
include::../common/resources/articles.adoc[]
include::../common/resources/standards-mobile.adoc[]
ifdef::env-github,rspecator-view[]
@ -20,4 +20,5 @@ ifdef::env-github,rspecator-view[]
include::../message.adoc[]
endif::env-github,rspecator-view[]

View File

@ -1,5 +1,5 @@
{
"title": "Exposing Java objects through JavaScript interfaces is security-sensitive",
"title": "Exposing Java interfaces in WebViews is security-sensitive",
"type": "SECURITY_HOTSPOT",
"status": "ready",
"remediation": {

View File

@ -1,15 +1,15 @@
Using JavaScript interfaces in WebViews to expose Java objects is unsafe. Doing so allows JavaScript
to invoke Java methods, potentially giving attackers access to data or sensitive app functionality.
WebViews might include untrusted sources such as third-party iframes, making this functionality
particularly risky. As JavaScript interfaces are passed to every frame in the WebView, those iframes
are also able to access the exposed Java object.
Using Javascript interfaces in WebViews is unsafe as it allows JavaScript to invoke Java methods,
potentially giving attackers access to data or sensitive app functionality. WebViews might include
untrusted sources such as third-party iframes, making this functionality particularly risky. As
Javascript interfaces are passed to every frame in the WebView, those iframes are also able to
access the exposed Java methods.
== Ask Yourself Whether
* The content in the WebView is fully trusted and secure.
* Potentially untrusted iframes could be loaded in the WebView.
* The JavaScript interface has to be exposed for the entire lifecycle of the WebView.
* The exposed Java object might be called by untrusted sources.
* The Javascript interface has to be exposed for the entire lifecycle of the WebView.
* The exposed Java methods will accept input from potentially untrusted sources.
There is a risk if you answered yes to any of these questions.
@ -18,9 +18,9 @@ There is a risk if you answered yes to any of these questions.
=== Disable JavaScript
If it is possible to disable JavaScript in the WebView, this is the most secure option. By default,
JavaScript is disabled in a WebView, so ``webSettings.setJavaScriptEnabled(false)`` does not need to
be explicitly called. Of course, sometimes it is necessary to enable JavaScript, in which case the
following recommendations should be considered.
JavaScript is disabled in a WebView, so you do not need to explicitly call
``webSettings.setJavaScriptEnabled(true)`` in your ``WebSettings`` configuration. Of course, sometimes
it is necessary to enable JavaScript, in which case the following recommendations should be considered.
=== Remove JavaScript interface when loading untrusted content
@ -63,8 +63,7 @@ class ExampleActivity : AppCompatActivity() {
== Compliant Solution
The most secure option is to disable JavaScript entirely. S6362 further explains why it should not be enabled
unless absolutely necessary.
The most secure option is to disable JavaScript entirely.
[source,kotlin]
----
@ -97,8 +96,7 @@ class ExampleActivity : AppCompatActivity() {
}
----
If a JavaScript bridge must be used, consider using ``WebViewCompat.addWebMessageListener`` instead. This allows you to restrict
the origins that can send messages to the JavaScript bridge.
If a JavaScript bridge must be used, consider using ``WebViewCompat.addWebMessageListener`` instead. This allows you to restrict the origins that can send messages to the JavaScript bridge.
[source,kotlin]
----
@ -137,6 +135,3 @@ class ExampleActivity : AppCompatActivity() {
* OWASP - https://owasp.org/www-project-mobile-top-10/2023-risks/m4-insufficient-input-output-validation.html[Mobile Top 10 2024 Category M4 - Insufficient Input/Output Validation]
* OWASP - https://owasp.org/www-project-mobile-top-10/2023-risks/m8-security-misconfiguration.html[Mobile Top 10 2024 Category M8 - Security Misconfiguration]
* CWE - https://cwe.mitre.org/data/definitions/79[CWE-79 - Improper Neutralization of Input During Web Page Generation]
=== Related rules
* S6362 - Enabling JavaScript support for WebViews is security-sensitive