51369b610e
When an include is not surrounded by empty lines, its content is inlined on the same line as the adjacent content. That can lead to broken tags and other display issues. This PR fixes all such includes and introduces a validation step that forbids introducing the same problem again.
96 lines
2.6 KiB
Plaintext
96 lines
2.6 KiB
Plaintext
== How to fix it in Java SE
|
|
|
|
=== Code examples
|
|
|
|
:code_impact: delete
|
|
|
|
include::../../common/fix/code-rationale.adoc[]
|
|
|
|
==== Noncompliant code example
|
|
|
|
[source,java,diff-id=1,diff-type=noncompliant]
|
|
----
|
|
@Controller
|
|
public class ExampleController
|
|
{
|
|
static private String targetDirectory = "/path/to/target/directory/";
|
|
|
|
@GetMapping(value = "/delete")
|
|
public void delete(@RequestParam("filename") String filename) throws IOException {
|
|
|
|
File file = new File(targetDirectory + filename);
|
|
file.delete();
|
|
}
|
|
}
|
|
----
|
|
|
|
==== Compliant solution
|
|
|
|
[source,java,diff-id=1,diff-type=compliant]
|
|
----
|
|
@Controller
|
|
public class ExampleController
|
|
{
|
|
static private String targetDirectory = "/path/to/target/directory/";
|
|
|
|
@GetMapping(value = "/delete")
|
|
public void delete(@RequestParam("filename") String filename) throws IOException {
|
|
|
|
File file = new File(targetDirectory + filename);
|
|
String canonicalDestinationPath = file.getCanonicalPath();
|
|
|
|
if (!canonicalDestinationPath.startsWith(targetDirectory)) {
|
|
throw new 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, java]
|
|
----
|
|
static private String targetDirectory = "/Users/John";
|
|
|
|
@GetMapping(value = "/endpoint")
|
|
public void endpoint(@RequestParam("folder") fileName) throws IOException {
|
|
|
|
String canonicalizedFileName = fileName.getCanonicalPath();
|
|
|
|
if (!canonicalizedFileName .startsWith(targetDirectory)) {
|
|
throw new IOException("Entry is outside of the target directory");
|
|
}
|
|
}
|
|
----
|
|
|
|
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[]
|