[APPSEC-48] Modify rule S2083[java]: Educational content (#1112)
This commit is contained in:
parent
47ba59f3b5
commit
9d944403b4
@ -1,3 +0,0 @@
|
||||
{
|
||||
|
||||
}
|
@ -1,16 +0,0 @@
|
||||
include::../rule.adoc[]
|
||||
|
||||
ifdef::env-github,rspecator-view[]
|
||||
|
||||
'''
|
||||
== Implementation Specification
|
||||
(visible only on this page)
|
||||
|
||||
include::../message.adoc[]
|
||||
|
||||
'''
|
||||
== Comments And Links
|
||||
(visible only on this page)
|
||||
|
||||
include::../comments-and-links.adoc[]
|
||||
endif::env-github,rspecator-view[]
|
@ -1,3 +0,0 @@
|
||||
{
|
||||
|
||||
}
|
@ -1,16 +0,0 @@
|
||||
include::../rule.adoc[]
|
||||
|
||||
ifdef::env-github,rspecator-view[]
|
||||
|
||||
'''
|
||||
== Implementation Specification
|
||||
(visible only on this page)
|
||||
|
||||
include::../message.adoc[]
|
||||
|
||||
'''
|
||||
== Comments And Links
|
||||
(visible only on this page)
|
||||
|
||||
include::../comments-and-links.adoc[]
|
||||
endif::env-github,rspecator-view[]
|
@ -1,3 +0,0 @@
|
||||
{
|
||||
|
||||
}
|
@ -1,16 +0,0 @@
|
||||
include::../rule.adoc[]
|
||||
|
||||
ifdef::env-github,rspecator-view[]
|
||||
|
||||
'''
|
||||
== Implementation Specification
|
||||
(visible only on this page)
|
||||
|
||||
include::../message.adoc[]
|
||||
|
||||
'''
|
||||
== Comments And Links
|
||||
(visible only on this page)
|
||||
|
||||
include::../comments-and-links.adoc[]
|
||||
endif::env-github,rspecator-view[]
|
8
rules/S2083/common/resources/standards.adoc
Normal file
8
rules/S2083/common/resources/standards.adoc
Normal file
@ -0,0 +1,8 @@
|
||||
=== Standards
|
||||
|
||||
* https://owasp.org/Top10/A01_2021-Broken_Access_Control/[OWASP Top 10 2021 Category A1] - Broken Access Control
|
||||
* https://owasp.org/Top10/A03_2021-Injection/[OWASP Top 10 2021 Category A3] - Injection
|
||||
* https://www.owasp.org/index.php/Top_10-2017_A1-Injection[OWASP Top 10 2017 Category A1] - Injection
|
||||
* https://www.owasp.org/index.php/Top_10-2017_A5-Broken_Access_Control[OWASP Top 10 2017 Category A5] - Broken Access Control
|
||||
* https://cwe.mitre.org/data/definitions/20[MITRE, CWE-20] - Improper Input Validation
|
||||
* https://cwe.mitre.org/data/definitions/22[MITRE, CWE-22] - Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')
|
@ -1,70 +1,14 @@
|
||||
include::../description.adoc[]
|
||||
== Why is this an issue?
|
||||
|
||||
== Noncompliant Code Example
|
||||
include::../rationale.adoc[]
|
||||
|
||||
[source,csharp]
|
||||
----
|
||||
using System;
|
||||
using Microsoft.AspNetCore.Mvc;
|
||||
include::../impact.adoc[]
|
||||
|
||||
namespace WebApplicationDotNetCore.Controllers
|
||||
{
|
||||
public class RSPEC2083IOInjectionNoncompliantController : Controller
|
||||
{
|
||||
public IActionResult Index()
|
||||
{
|
||||
return View();
|
||||
}
|
||||
== How to fix it?
|
||||
|
||||
public IActionResult DeleteFile(string fileName)
|
||||
{
|
||||
System.IO.File.Delete(fileName); // Noncompliant
|
||||
== Resources
|
||||
|
||||
return Content("File " + fileName + " deleted");
|
||||
}
|
||||
}
|
||||
}
|
||||
----
|
||||
|
||||
== Compliant Solution
|
||||
|
||||
[source,csharp]
|
||||
----
|
||||
using System;
|
||||
using System.IO;
|
||||
using Microsoft.AspNetCore.Mvc;
|
||||
|
||||
namespace WebApplicationDotNetCore.Controllers
|
||||
{
|
||||
public class RSPEC2083IOInjectionCompliantController : Controller
|
||||
{
|
||||
public IActionResult Index()
|
||||
{
|
||||
return View();
|
||||
}
|
||||
|
||||
public IActionResult DeleteFile(string fileName)
|
||||
{
|
||||
string destDirectory = "~/CustomersData/";
|
||||
|
||||
string destFileName = Path.GetFullPath(System.IO.Path.Combine(destDirectory, fileName));
|
||||
string fullDestDirPath = Path.GetFullPath(destDirectory + Path.DirectorySeparatorChar);
|
||||
|
||||
if (destFileName.StartsWith(fullDestDirPath, StringComparison.Ordinal))
|
||||
{
|
||||
System.IO.File.Delete(destFileName); // Compliant
|
||||
return Content("File " + fileName + " deleted");
|
||||
} else
|
||||
{
|
||||
return BadRequest();
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
}
|
||||
----
|
||||
|
||||
include::../see.adoc[]
|
||||
include::../common/resources/standards.adoc[]
|
||||
|
||||
ifdef::env-github,rspecator-view[]
|
||||
|
||||
|
@ -1,7 +0,0 @@
|
||||
User-provided data, such as URL parameters, POST data payloads, or cookies, should always be considered untrusted and tainted. Constructing file system paths directly from tainted data could enable an attacker to inject specially crafted values, such as ``++'../'++``, that change the initial path and, when accessed, resolve to a path on the filesystem where the user should normally not have access.
|
||||
|
||||
|
||||
A successful attack might give an attacker the ability to read, modify, or delete sensitive information from the file system and sometimes even execute arbitrary operating system commands. This is often referred to as a "path traversal" or "directory traversal" attack.
|
||||
|
||||
|
||||
The mitigation strategy should be based on the whitelisting of allowed paths or characters.
|
17
rules/S2083/impact.adoc
Normal file
17
rules/S2083/impact.adoc
Normal file
@ -0,0 +1,17 @@
|
||||
=== What is the potential impact?
|
||||
|
||||
A web application is vulnerable to path injection and an attacker is able to exploit it.
|
||||
|
||||
The files that can be affected are limited by the permission of the process that runs the application. Worst case scenario: the process runs with root privileges on Linux, and therefore any file can be affected.
|
||||
|
||||
Below are some real-world scenarios that illustrate the various effects of an
|
||||
attacker exploiting the vulnerability.
|
||||
|
||||
==== Override or delete arbitrary files
|
||||
|
||||
The injected path component tampers with the location of a file the application is supposed to delete or write into. The vulnerability is exploited to remove or corrupt files that are critical for the application or for the system to work properly.
|
||||
It could result in data being lost or the application being unavailable.
|
||||
|
||||
==== Read arbitrary files
|
||||
|
||||
The injected path component tampers with the location of a file the application is supposed to read and output. The vulnerability is exploited to leak the content of arbitrary files from the file system, including sensitive files like SSH private keys.
|
56
rules/S2083/java/how-to-fix-it/java-se.adoc
Normal file
56
rules/S2083/java/how-to-fix-it/java-se.adoc
Normal file
@ -0,0 +1,56 @@
|
||||
=== How to fix it in Java SE
|
||||
|
||||
The following code is vulnerable to path injection as it is constructing a path using untrusted data. This path is then used to delete a file without being validated first. Therefore, it can be leveraged by an attacker to delete arbitrary files.
|
||||
|
||||
[cols="a"]
|
||||
|===
|
||||
h| Non-compliant code example
|
||||
|
|
||||
[source,java]
|
||||
----
|
||||
@RestController
|
||||
public class ApiController
|
||||
{
|
||||
static private String targetDirectory = "/path/to/target/directory/";
|
||||
|
||||
@GetMapping(value = "/endpoint")
|
||||
public void endpoint(@RequestParam("filename") filename) throws IOException {
|
||||
|
||||
File file = new File(targetDirectory + filename);
|
||||
file.delete(); // Noncompliant
|
||||
}
|
||||
}
|
||||
----
|
||||
h| Compliant solution
|
||||
|
|
||||
[source,java]
|
||||
----
|
||||
@RestController
|
||||
public class ApiController
|
||||
{
|
||||
static private String targetDirectory = "/path/to/target/directory/";
|
||||
|
||||
@GetMapping(value = "/endpoint")
|
||||
public void endpoint(@RequestParam("filename") 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?
|
||||
|
||||
The universal way to prevent path injection is to validate paths constructed from untrusted data.
|
||||
|
||||
The validation should be done as follow:
|
||||
|
||||
1. Resolve the canonical path of the file by using methods like java.io.File/getCanonicalPath. This will resolve relative path or path components like `../` and removes any ambiguity regarding the file's location.
|
||||
2. Check that the canonical path is within the directory where the file should be located.
|
@ -1,44 +1,16 @@
|
||||
include::../description.adoc[]
|
||||
== Why is this an issue?
|
||||
|
||||
== Noncompliant Code Example
|
||||
include::../rationale.adoc[]
|
||||
|
||||
[source,java]
|
||||
----
|
||||
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
|
||||
String file = request.getParameter("file");
|
||||
include::../impact.adoc[]
|
||||
|
||||
File fileUnsafe = new File(file);
|
||||
try {
|
||||
FileUtils.forceDelete(fileUnsafe); // Noncompliant
|
||||
}
|
||||
catch(IOException ex){
|
||||
System.out.println (ex.toString());
|
||||
}
|
||||
}
|
||||
----
|
||||
== How to fix it?
|
||||
|
||||
== Compliant Solution
|
||||
include::how-to-fix-it/java-se.adoc[]
|
||||
|
||||
[source,java]
|
||||
----
|
||||
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
|
||||
String file = request.getParameter("file");
|
||||
== Resources
|
||||
|
||||
File fileUnsafe = new File(file);
|
||||
File directory = new File("/tmp/");
|
||||
|
||||
try {
|
||||
if(FileUtils.directoryContains(directory, fileUnsafe)) {
|
||||
FileUtils.forceDelete(fileUnsafe); // Compliant
|
||||
}
|
||||
}
|
||||
catch(IOException ex){
|
||||
System.out.println (ex.toString());
|
||||
}
|
||||
}
|
||||
----
|
||||
|
||||
include::../see.adoc[]
|
||||
include::../common/resources/standards.adoc[]
|
||||
|
||||
ifdef::env-github,rspecator-view[]
|
||||
|
||||
|
@ -1,3 +0,0 @@
|
||||
{
|
||||
|
||||
}
|
@ -1,48 +0,0 @@
|
||||
include::../description.adoc[]
|
||||
|
||||
== Noncompliant Code Example
|
||||
|
||||
[source,javascript]
|
||||
----
|
||||
const fs = require('fs');
|
||||
|
||||
function (req, res) {
|
||||
const reqPath = __dirname + req.query.filename; // user-controlled path
|
||||
|
||||
let data = fs.readFileSync(reqPath, { encoding: 'utf8', flag: 'r' }); // Noncompliant
|
||||
}
|
||||
----
|
||||
|
||||
== Compliant Solution
|
||||
|
||||
[source,javascript]
|
||||
----
|
||||
const fs = require('fs');
|
||||
const pathmodule = require('path');
|
||||
|
||||
function (req, res) {
|
||||
const reqPath = __dirname + req.query.filename; // user-controlled path
|
||||
const resolvedPath = pathmodule.resolve(reqPath); // resolve will resolve "../"
|
||||
|
||||
if (resolvedPath.startsWith(__dirname + '/uploads')) { // the requested filename cannot be retrieved outside of the "/uploads" folder
|
||||
let data = fs.readFileSync(resolvedPath, { encoding: 'utf8', flag: 'r' }); // Compliant
|
||||
}
|
||||
}
|
||||
----
|
||||
|
||||
include::../see.adoc[]
|
||||
|
||||
ifdef::env-github,rspecator-view[]
|
||||
|
||||
'''
|
||||
== Implementation Specification
|
||||
(visible only on this page)
|
||||
|
||||
include::../message.adoc[]
|
||||
|
||||
'''
|
||||
== Comments And Links
|
||||
(visible only on this page)
|
||||
|
||||
include::../comments-and-links.adoc[]
|
||||
endif::env-github,rspecator-view[]
|
@ -1,3 +0,0 @@
|
||||
{
|
||||
|
||||
}
|
@ -1,42 +0,0 @@
|
||||
include::../description.adoc[]
|
||||
|
||||
== Noncompliant Code Example
|
||||
|
||||
[source,php]
|
||||
----
|
||||
$userId = $_GET["userId"];
|
||||
$fileUUID = $_GET["fileUUID"];
|
||||
|
||||
if ( $_SESSION["userId"] == $userId ) {
|
||||
unlink("/storage/" . $userId . "/" . $fileUUID); // Noncompliant
|
||||
}
|
||||
----
|
||||
|
||||
== Compliant Solution
|
||||
|
||||
[source,php]
|
||||
----
|
||||
$userId = (int) $_GET["userId"];
|
||||
$fileUUID = (int) $_GET["fileUUID"];
|
||||
|
||||
if ( $_SESSION["userId"] == $userId ) {
|
||||
unlink("/storage/" . $userId . "/" . $fileUUID);
|
||||
}
|
||||
----
|
||||
|
||||
include::../see.adoc[]
|
||||
|
||||
ifdef::env-github,rspecator-view[]
|
||||
|
||||
'''
|
||||
== Implementation Specification
|
||||
(visible only on this page)
|
||||
|
||||
include::../message.adoc[]
|
||||
|
||||
'''
|
||||
== Comments And Links
|
||||
(visible only on this page)
|
||||
|
||||
include::../comments-and-links.adoc[]
|
||||
endif::env-github,rspecator-view[]
|
@ -1,3 +0,0 @@
|
||||
{
|
||||
|
||||
}
|
@ -1,42 +0,0 @@
|
||||
include::../description.adoc[]
|
||||
|
||||
== Noncompliant Code Example
|
||||
|
||||
[source,python]
|
||||
----
|
||||
from flask import request, send_file
|
||||
|
||||
@app.route('/download')
|
||||
def download():
|
||||
file = request.args['file']
|
||||
return send_file("static/%s" % file, as_attachment=True) # Noncompliant
|
||||
----
|
||||
|
||||
== Compliant Solution
|
||||
|
||||
[source,python]
|
||||
----
|
||||
from flask import request, send_from_directory
|
||||
|
||||
@app.route('/download')
|
||||
def download():
|
||||
file = request.args['file']
|
||||
return send_from_directory('static', file) # Compliant
|
||||
----
|
||||
|
||||
include::../see.adoc[]
|
||||
|
||||
ifdef::env-github,rspecator-view[]
|
||||
|
||||
'''
|
||||
== Implementation Specification
|
||||
(visible only on this page)
|
||||
|
||||
include::../message.adoc[]
|
||||
|
||||
'''
|
||||
== Comments And Links
|
||||
(visible only on this page)
|
||||
|
||||
include::../comments-and-links.adoc[]
|
||||
endif::env-github,rspecator-view[]
|
4
rules/S2083/rationale.adoc
Normal file
4
rules/S2083/rationale.adoc
Normal file
@ -0,0 +1,4 @@
|
||||
Path injections occur when an application uses untrusted data to construct a file path and access this file without validating its path first.
|
||||
|
||||
A user with malicious intent would inject specially crafted values, such as ``++../++``, to change the initial intended path. The resulting path would resolve somewhere in the filesystem where the user should not normally have access to.
|
||||
|
@ -1,3 +0,0 @@
|
||||
include::description.adoc[]
|
||||
|
||||
include::see.adoc[]
|
@ -1,11 +0,0 @@
|
||||
== See
|
||||
|
||||
* https://owasp.org/Top10/A01_2021-Broken_Access_Control/[OWASP Top 10 2021 Category A1] - Broken Access Control
|
||||
* https://owasp.org/Top10/A03_2021-Injection/[OWASP Top 10 2021 Category A3] - Injection
|
||||
* https://owasp.org/www-project-top-ten/2017/A1_2017-Injection[OWASP Top 10 2017 Category A1] - Injection
|
||||
* https://owasp.org/www-project-top-ten/2017/A5_2017-Broken_Access_Control[OWASP Top 10 2017 Category A5] - Broken Access Control
|
||||
* https://cwe.mitre.org/data/definitions/20[MITRE, CWE-20] - Improper Input Validation
|
||||
* https://cwe.mitre.org/data/definitions/22[MITRE, CWE-22] - Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')
|
||||
* https://cwe.mitre.org/data/definitions/99[MITRE, CWE-99] - Improper Control of Resource Identifiers ('Resource Injection')
|
||||
* https://cwe.mitre.org/data/definitions/641[MITRE, CWE-641] - Improper Restriction of Names for Files and Other Resources
|
||||
* https://www.sans.org/top25-software-errors/#cat2[SANS Top 25] - Risky Resource Management
|
@ -1,3 +0,0 @@
|
||||
{
|
||||
|
||||
}
|
@ -1,16 +0,0 @@
|
||||
include::../rule.adoc[]
|
||||
|
||||
ifdef::env-github,rspecator-view[]
|
||||
|
||||
'''
|
||||
== Implementation Specification
|
||||
(visible only on this page)
|
||||
|
||||
include::../message.adoc[]
|
||||
|
||||
'''
|
||||
== Comments And Links
|
||||
(visible only on this page)
|
||||
|
||||
include::../comments-and-links.adoc[]
|
||||
endif::env-github,rspecator-view[]
|
@ -1,3 +0,0 @@
|
||||
{
|
||||
|
||||
}
|
@ -1,16 +0,0 @@
|
||||
include::../rule.adoc[]
|
||||
|
||||
ifdef::env-github,rspecator-view[]
|
||||
|
||||
'''
|
||||
== Implementation Specification
|
||||
(visible only on this page)
|
||||
|
||||
include::../message.adoc[]
|
||||
|
||||
'''
|
||||
== Comments And Links
|
||||
(visible only on this page)
|
||||
|
||||
include::../comments-and-links.adoc[]
|
||||
endif::env-github,rspecator-view[]
|
Loading…
x
Reference in New Issue
Block a user