Modify S2083(multiple languages): Update to the education framework (APPSEC-188) (#1328)

docs/allowed_framework_names.adoc

@@ -15,6 +15,7 @@
 * Apache Commons
 * Legacy Mongo Java API
 // JS
+* Node.js
 * Express.js
 // PHP
 * Core PHP

rules/S2083/common/fix/code-rationale.adoc

@@ -1 +1,5 @@
-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.
+The following code is vulnerable to path injection as it creates a path using
+untrusted data without validation.
+
+An attacker can exploit the vulnerability in this code to {code_impact}
+arbitrary files.

rules/S2083/common/fix/function-based-validation.adoc

@@ -0,0 +1,14 @@
+==== Use secure-by-design APIs
+
+Some libraries contain APIs with these three capabilities:
+
+* File retrieval in a file system.
+* Restriction of the file retrieval to a specific folder (thus sanitizing and validating untrusted data).
+* A feature, such as a file download or file deletion.
+
+They can be referred to as "secure-by-design" APIs. Using this type of API,
+such as '{auto_canonicalization_function}', brings multiple layers of security
+to the code while keeping the code base shorter.
+
+Behind the scenes, this function protects against both regular and partial path
+injection.

rules/S2083/common/fix/how-does-this-work.adoc

@@ -1,7 +0,0 @@
-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 `{canonicalization_function}`. 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.
-3. Ensure the target directory path ends with a forward slash to prevent partial path traversal, for example, */base/dirmalicious* starts with */base/dir* but does not start with */base/dir/*.

rules/S2083/common/fix/self-validation.adoc

@@ -0,0 +1,20 @@
+==== Canonical path validation
+
+If it is impossible to use secure-by-design APIs that do this automatically, the universal way to prevent path injection is to validate paths constructed from untrusted data:
+
+1. Ensure the target directory path ends with a forward slash to prevent partial path traversal, for example, `/base/dirmalicious` starts with `/base/dir` but does not start with `/base/dir/`.
+2. Resolve the canonical path of the file by using methods like `{canonicalization_function}`. This will resolve relative path or path components like `../` and removes any ambiguity regarding the file's location. 
+3. Check that the canonical path is within the directory where the file should be located.
+
+*Important Note*: The order of this process pattern is important. The code must
+follow this order exactly to be secure by design:
+
+1. `data = transform(user_input);`
+2. `data = normalize(data);`
+3. `data = sanitize(data);`
+4. `use(data);`
+
+:tnsu_talk: https://www.youtube.com/watch?v=V-DdcKADnFk
+As pointed out in {tnsu_talk}[this SonarSource talk], failure to follow this
+exact order leads to security vulnerabilities.
+

rules/S2083/common/pitfalls/python-path-join.adoc

@@ -0,0 +1,12 @@
+==== os.path.join(path, \*paths)
+
+This function should not be used as a validator.
+
+The standard library states: *"if a component is an absolute path, all previous
+components are discarded, and linking continues from the component with the
+absolute path."*
+
+This means that including untrusted data in any of the path parameters can lead
+to a full or partial path traversal vulnerability.
+
+https://blog.sonarsource.com/10-unknown-security-pitfalls-for-python/[This Sonar blog post] talks about this issue.

rules/S2083/csharp/how-to-fix-it/dotnet.adoc

@@ -1,6 +1,6 @@
 === How to fix it in .NET
 
-:canonicalization_function: System.IO.Path.GetFullPath
+:code_impact: delete
 include::../../common/fix/code-rationale.adoc[]
 
 ==== Noncompliant code example
@@ -42,7 +42,8 @@ public class ExampleController : Controller
 
 === How does this work?
 
-include::../../common/fix/how-does-this-work.adoc[]
+:canonicalization_function: System.IO.Path.GetFullPath
+include::../../common/fix/self-validation.adoc[]
 
 === Pitfalls
 

rules/S2083/java/how-to-fix-it/java-se.adoc

@@ -1,6 +1,6 @@
 === How to fix it in Java SE
 
-:canonicalization_function: java.io.File.getCanonicalPath
+:code_impact: delete
 include::../../common/fix/code-rationale.adoc[]
 
 ==== Noncompliant code example
@@ -47,7 +47,8 @@ public class ExampleController
 
 === How does this work?
 
-include::../../common/fix/how-does-this-work.adoc[]
+:canonicalization_function: java.io.File.getCanonicalPath
+include::../../common/fix/self-validation.adoc[]
 
 === Pitfalls
 

rules/S2083/javascript/how-to-fix-it/express.adoc

@@ -0,0 +1,40 @@
+=== How to fix it in Express.js
+
+:code_impact: read
+include::../../common/fix/code-rationale.adoc[]
+
+==== Noncompliant code example
+
+[source,javascript,diff-id=1,diff-type=noncompliant]
+----
+const path = require('path');
+
+function (req, res) {
+  const targetDirectory = "/data/app/resources/"
+  const userFilename = path.join(targetDirectory, req.query.filename);
+
+  res.sendFile(userFilename); // Noncompliant
+}
+----
+
+==== Compliant solution
+
+[source,javascript,diff-id=1,diff-type=compliant]
+----
+const path = require('path');
+
+function (req, res) {
+  const targetDirectory = "/data/app/resources/";
+  const userFilename = req.query.filename;
+
+  res.sendFile(userFilename, { root: targetDirectory });
+}
+----
+
+=== How does this work?
+
+:auto_canonicalization_function: res.sendFile() (used with options.root)
+
+include::../../common/fix/function-based-validation.adoc[]
+
+include::../../common/fix/self-validation.adoc[]

rules/S2083/javascript/how-to-fix-it/node.adoc

@@ -0,0 +1,77 @@
+=== How to fix it in Node.js
+
+:code_impact: read
+include::../../common/fix/code-rationale.adoc[]
+
+==== Noncompliant code example
+
+[source,javascript,diff-id=1,diff-type=noncompliant]
+----
+const path = require('path');
+
+function (req, res) {
+  const targetDirectory = "/data/app/resources/"
+  const userFilename = path.join(targetDirectory, req.query.filename);
+
+  let data = fs.readFileSync(userFilename, { encoding: 'utf8', flag: 'r' }); // Noncompliant
+}
+----
+
+==== Compliant solution
+
+[source,javascript,diff-id=1,diff-type=compliant]
+----
+const path = require('path');
+
+function (req, res) {
+  const targetDirectory = "/data/app/resources/"
+  const userFilename = path.join(targetDirectory, req.query.filename));
+  const userFilename = fs.realPath(userFilename);
+
+  if (!userFilename.startsWith(targetDirectory)) {
+    res.status(401).send();
+  }
+
+  let data = fs.readFileSync(userFilename, { encoding: 'utf8', flag: 'r' });
+}
+----
+
+=== How does this work?
+
+:canonicalization_function: `fs.realPath`
+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, javascript]
+----
+const path = require('path');
+
+function (req, res) {
+  const targetDirectory = "/data/app/resources"
+  const userFilename = path.join(targetDirectory, req.query.filename));
+  const userFilename = fs.realPath(userFilename);
+
+  if (!userFilename.startsWith(targetDirectory)) {
+    res.status(401).send();
+  }
+
+  let data = fs.readFileSync(userFilename);
+}
+----
+
+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 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.
+

rules/S2083/javascript/metadata.json

@@ -0,0 +1,3 @@
+{
+  
+}

rules/S2083/javascript/rule.adoc

@@ -0,0 +1,30 @@
+== Why is this an issue?
+
+include::../rationale.adoc[]
+
+include::../impact.adoc[]
+
+== How to fix it?
+
+include::how-to-fix-it/node.adoc[]
+
+include::how-to-fix-it/express.adoc[]
+
+== Resources
+
+include::../common/resources/standards.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[]

rules/S2083/php/how-to-fix-it/core.adoc

@@ -0,0 +1,33 @@
+=== How to fix it in Core PHP
+
+:code_impact: read
+include::../../common/fix/code-rationale.adoc[]
+
+==== Noncompliant code example
+
+[source,php,diff-id=1,diff-type=noncompliant]
+----
+$fileName = $_GET["filename"];
+
+file_get_contents($fileName); // Noncompliant
+----
+
+==== Compliant solution
+
+[source,php,diff-id=1,diff-type=compliant]
+----
+$fileName = $_GET["filename"];
+$targetDirectory = "/path/to/target/directory/";
+
+$path = realpath($targetDirectory . $fileName);
+
+if (str_starts_with($path,  $targetDirectory)) {
+    file_get_contents($path);
+}
+----
+
+=== How does this work?
+
+:canonicalization_function: `realPath`
+include::../../common/fix/self-validation.adoc[]
+

rules/S2083/php/metadata.json

@@ -0,0 +1,3 @@
+{
+  
+}

rules/S2083/php/rule.adoc

@@ -0,0 +1,28 @@
+== Why is this an issue?
+
+include::../rationale.adoc[]
+
+include::../impact.adoc[]
+
+== How to fix it?
+
+include::how-to-fix-it/core.adoc[]
+
+== Resources
+
+include::../common/resources/standards.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[]

rules/S2083/python/how-to-fix-it/flask.adoc

@@ -0,0 +1,50 @@
+=== How to fix it in Flask
+
+:code_impact: read
+include::../../common/fix/code-rationale.adoc[]
+
+==== Noncompliant code example
+
+[source,python,diff-id=1,diff-type=noncompliant]
+----
+from flask import Flask, request, send_from_directory
+
+app = Flask('example')
+
+@app.route('/example')
+def example():
+    my_file = request.args['my_file']
+    return send_file("static/%s" % my_file, as_attachment=True) # Noncompliant
+----
+
+==== Compliant solution
+
+[source,python,diff-id=1,diff-type=compliant]
+----
+from flask import Flask, request, send_from_directory
+
+app = Flask('example')
+
+@app.route('/example')
+def example():
+    my_file = request.args['my_file']
+    return send_from_directory('static', my_file)
+----
+
+=== How does this work?
+
+:auto_canonicalization_function: send_from_directory
+
+The universal method to prevent path injection is to validate paths created
+from untrusted data. This can be done either manually or automatically,
+depending on whether the library includes a data sanitization feature and the
+required function.
+
+Here, {auto_canonicalization_function} can be considered a secure-by-design API.
+
+include::../../common/fix/function-based-validation.adoc[]
+
+=== Pitfalls
+
+include::../../common/pitfalls/python-path-join.adoc[]
+

rules/S2083/python/metadata.json

@@ -0,0 +1,3 @@
+{
+  
+}

rules/S2083/python/rule.adoc

@@ -0,0 +1,28 @@
+== Why is this an issue?
+
+include::../rationale.adoc[]
+
+include::../impact.adoc[]
+
+== How to fix it?
+
+include::how-to-fix-it/flask.adoc[]
+
+== Resources
+
+include::../common/resources/standards.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[]