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
5 changed files with 158 additions and 7 deletions

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

@ -0,0 +1,33 @@
{
"securityStandards": {
"CWE": [
20,
22
],
"OWASP": [
"A5",
"A1"
],
"OWASP Top 10 2021": [
"A1",
"A3"
],
"OWASP Mobile Top 10 2024": [
"M4"
],
"PCI DSS 3.2": [
"6.5.8"
],
"PCI DSS 4.0": [
"6.2.4"
],
"ASVS 4.0": [
"12.3.1",
"5.1.3",
"5.1.4"
],
"STIG ASD_V5R3": [
"V-222609"
]
}
}

View File

@ -0,0 +1,24 @@
== Why is this an issue?
include::../rationale.adoc[]
include::../impact.adoc[]
// How to fix it section
include::how-to-fix-it/java-io-api.adoc[]
== Resources
include::../common/resources/standards-mobile.adoc[]
ifdef::env-github,rspecator-view[]
'''
== Implementation Specification
(visible only on this page)
include::../message.adoc[]
endif::env-github,rspecator-view[]