4e3c0d465a
* Add kotlin to rule S2083 * Add Kotlin rule description, update Java SE name, minor java fix * Apply review comments --------- Co-authored-by: christophe-zurn-sonarsource <christophe-zurn-sonarsource@users.noreply.github.com> Co-authored-by: Christophe Zurn <christophe.zurn@sonarsource.com>
95 lines
2.6 KiB
Plaintext
95 lines
2.6 KiB
Plaintext
== How to fix it in Java I/O API
|
|
|
|
=== Code examples
|
|
|
|
:code_impact: delete
|
|
|
|
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("/delete")
|
|
fun delete(@RequestParam("filename") filename: String) {
|
|
val file = File(TARGET_DIRECTORY + filename)
|
|
file.delete()
|
|
}
|
|
}
|
|
----
|
|
|
|
==== Compliant solution
|
|
|
|
[source,kotlin,diff-id=1,diff-type=compliant]
|
|
----
|
|
@Controller
|
|
class ExampleController {
|
|
companion object {
|
|
private const val TARGET_DIRECTORY = "/path/to/target/directory/"
|
|
private val TARGET_PATH = File(TARGET_DIRECTORY).toPath().normalize()
|
|
}
|
|
|
|
@GetMapping("/delete")
|
|
fun delete(@RequestParam("filename") filename: String) {
|
|
val file = File(TARGET_PATH.toString() + filename)
|
|
if (!file.toPath().normalize().startsWith(TARGET_PATH)) {
|
|
throw IOException("Entry is outside of the target directory")
|
|
}
|
|
file.delete()
|
|
}
|
|
}
|
|
----
|
|
|
|
=== How does this work?
|
|
|
|
:canonicalization_function: java.io.File.getCanonicalPath
|
|
|
|
include::../../common/fix/self-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 targetDirectory: String = "/Users/John"
|
|
}
|
|
|
|
@GetMapping("/endpoint")
|
|
fun endpoint(@RequestParam("folder") file: File) {
|
|
val canonicalizedFileName = file.getCanonicalPath()
|
|
if (!canonicalizedFileName .startsWith(targetDirectory)) {
|
|
throw IOException("Entry is outside of the target directory");
|
|
}
|
|
file.delete()
|
|
}
|
|
----
|
|
|
|
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.]
|
|
|
|
|
|
: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[]
|