Modify S2596(javascript): Convert to LayC (#2901)

This PR also removes the java folder because it is not implemented and
has no implementation plan. This PR was made spontaneously during
Daniel's onboarding.

---------

Co-authored-by: daniel-teuchert-sonarsource <141642369+daniel-teuchert-sonarsource@users.noreply.github.com>

docs/header_names/allowed_framework_names.adoc

@@ -54,6 +54,8 @@
 * DOM API
 * jsonwebtoken
 * libxmljs
+* Formidable
+* Multer
 // PHP
 * Core PHP
 * Guzzle

rules/S2598/comments-and-links.adoc

@@ -1,11 +0,0 @@
-=== on 21 Jan 2021, 15:37:26 Pierre-Loup Tristant wrote:
-This rule is likely not implementable for C#.
-
-ASP.NET Core is not providing any high level interface to help developper manage uploaded files.
-
-There is no temporary storage of uploaded file by default. The file stays in memory and it's up to the developper to chose the end location.
-
-Verifying file extention can be done in many different ways.
-
-
-

rules/S2598/common/fix/allowed-folder.adoc

@@ -0,0 +1,16 @@
+==== Use pre-approved folders
+
+Create a special folder where untrusted data should be stored. This folder
+should be classified as untrusted and have the following characteristics:
+
+* It should have specific read and write permissions that belong to the right people or organizations.
+* It should have a size limit or its size should be monitored.
+* It should contain backup copies if it contains data that belongs to users.
+
+This folder should not be located in `/tmp`, `/var/tmp` or in the Windows
+directory `%TEMP%`. +
+These folders are usually "world-writable", can be manipulated, and can be
+accidentally deleted by the system.
+
+Also, the original file names and extensions should be changed to controlled
+strings to prevent unwanted code from being executed based on the file names.

rules/S2598/description.adoc

@@ -1,6 +0,0 @@
-These minimum restrictions should be applied when handling file uploads:
-
-* the file upload folder to restrict untrusted files to a specific folder.
-* the file extension of the uploaded file to prevent remote code execution.
-
-Also the size of the uploaded file should be limited to prevent denial of service attacks. This requirement is covered by the rule S5693.

rules/S2598/impact.adoc

@@ -0,0 +1,43 @@
+
+=== What is the potential impact?
+
+After discovering this vulnerability, attackers may attempt to upload as many
+different file types as possible, such as javascript files, bash scripts,
+malware, or malicious configuration files targeting potential processes.
+
+Below are some real-world scenarios that illustrate the potential impact of an
+attacker exploiting the vulnerability.
+
+==== Full application compromise
+
+In the worst-case scenario, the attackers succeed in uploading a file recognized
+by in an internal tool, triggering code execution.
+
+Depending on the attacker, code execution can be used with different
+intentions:
+
+* Download the internal server's data, most likely to sell it.
+* Modify data, install malware, for instance, malware that mines cryptocurrencies.
+* Stop services or exhaust resources, for instance, with fork bombs.
+
+==== Server Resource Exhaustion
+
+By repeatedly uploading large files, an attacker can consume excessive server
+resources, resulting in a denial of service.
+
+If the component affected by this vulnerability is not a bottleneck that acts
+as a single point of failure (SPOF) within the application, the denial of
+service can only affect the attacker who caused it.
+
+Even though a denial of service might have little direct impact, it can have
+secondary impact in architectures that use containers and container
+orchestrators. For example, it can cause unexpected container failures or
+overuse of resources.
+
+In some cases, it is also possible to force the product to "fail open" when
+resources are exhausted, which means that some security features are disabled
+in an emergency.
+
+These threats are particularly insidious if the attacked organization does not
+maintain a disaster recovery plan (DRP).
+

rules/S2598/java/metadata.json

@@ -1,30 +0,0 @@
-{
-  "tags": [
-    "cwe",
-    "cert"
-  ],
-  "securityStandards": {
-    "CERT": [
-      "IDS56-J."
-    ],
-    "CWE": [
-      434,
-      400
-    ],
-    "OWASP Top 10 2021": [
-      "A4"
-    ],
-    "PCI DSS 3.2": [
-      "6.5.8"
-    ],
-    "PCI DSS 4.0": [
-      "6.2.4"
-    ],
-    "ASVS 4.0": [
-      "12.1.1",
-      "12.2.1",
-      "12.5.2",
-      "13.1.5"
-    ]
-  }
-}

rules/S2598/java/rule.adoc

@@ -1,36 +0,0 @@
-== Why is this an issue?
-
-include::../description.adoc[]
-
-=== Noncompliant code example
-
-[source,java]
-----
-public void doPost(HttpServletRequest request, HttpServletResponse response) throws IOException {
-
-  if (ServletFileUpload.isMultipartContent(request)) {
-
-    FileItemFactory factory = new DiskFileItemFactory();
-    ServletFileUpload upload = new ServletFileUpload(factory);  // Noncompliant
-    // ...
-----
-
-include::../see.adoc[]
-
-* https://wiki.sei.cmu.edu/confluence/display/java/IDS56-J.+Prevent+arbitrary+file+upload[CERT, IDS56-J.] - Prevent arbitrary file upload
-
-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/S2598/javascript/how-to-fix-it/formidable.adoc

@@ -0,0 +1,29 @@
+== How to fix it in Formidable
+
+=== Code examples
+
+==== Noncompliant code example
+
+[source,javascript,diff-id=1,diff-type=noncompliant]
+----
+const Formidable = require('formidable');
+
+const form          = new Formidable(); // Noncompliant
+form.uploadDir      = "/tmp/";
+form.keepExtensions = true;
+----
+
+==== Compliant solution
+
+[source,javascript,diff-id=1,diff-type=compliant]
+----
+const Formidable = require('formidable');
+
+const form          = new Formidable();
+form.uploadDir      = "/uploads/";
+form.keepExtensions = false;
+----
+
+=== How does this work?
+
+include::../../common/fix/allowed-folder.adoc[]

rules/S2598/javascript/how-to-fix-it/multer.adoc

@@ -0,0 +1,50 @@
+== How to fix it in Multer
+
+=== Code examples
+
+==== Noncompliant code example
+
+The following code sample is vulnerable because it implicitly uses `/tmp` or
+`/var/tmp` as upload directory.
+
+[source,javascript,diff-id=2,diff-type=noncompliant]
+----
+const crypto = require('node:crypto');
+const multer = require('multer');
+
+let diskStorage = multer.diskStorage({
+  filename: (req, file, cb) => {
+    const buf = crypto.randomBytes(20);
+    cb(null, buf.toString('hex'))
+  }
+}); // Noncompliant
+
+let diskUpload = multer({
+  storage: diskStorage,
+});
+----
+
+==== Compliant code example
+
+[source,javascript,diff-id=2,diff-type=compliant]
+----
+const multer = require('multer');
+
+let diskStorage = multer.diskStorage({
+  filename: (req, file, cb) => {
+    const buf = crypto.randomBytes(20);
+    cb(null, buf.toString('hex'))
+  },
+  destination: (req, file, cb) => {
+    cb(null, '/uploads/')
+  }
+});
+
+let diskUpload = multer({
+  storage: diskStorage,
+});
+----
+
+=== How does this work?
+
+include::../../common/fix/allowed-folder.adoc[]

rules/S2598/javascript/rule.adoc

@@ -1,74 +1,22 @@
+
+
 == Why is this an issue?
 
-include::../description.adoc[]
+include::../rationale.adoc[]
 
-=== Noncompliant code example
+include::../impact.adoc[]
 
-https://www.npmjs.com/package/formidable[formidable] module:
+include::how-to-fix-it/formidable.adoc[]
 
-[source,javascript]
+include::how-to-fix-it/multer.adoc[]
-----
-const Formidable = require('formidable');
 
-const form = new Formidable(); // Noncompliant, this form is not safe
+== Resources
-form.uploadDir = ""; // because upload dir is not defined (by default os temp dir: /var/tmp or /tmp)
-form.keepExtensions = true; // and file extensions are kept
-----
 
-https://www.npmjs.com/package/multer[multer] (Express.js middleware) module:
+* https://owasp.org/Top10/A04_2021-Insecure_Design/[OWASP Top 10 2021 Category A4] - Insecure Design
+* https://cwe.mitre.org/data/definitions/434[MITRE, CWE-434] - Unrestricted Upload of File with Dangerous Type
+* https://cwe.mitre.org/data/definitions/400[MITRE, CWE-400] - Uncontrolled Resource Consumption
+* https://owasp.org/www-community/vulnerabilities/Unrestricted_File_Upload[OWASP Unrestricted File Upload] - Unrestricted File Upload
 
-[source,javascript]
-----
-const multer = require('multer');
-
-let diskStorage = multer.diskStorage({ // Noncompliant: no destination specified
-  filename: (req, file, cb) => {
-    const buf = crypto.randomBytes(20);
-    cb(null, buf.toString('hex'))
-  }
-});
-
-// This upload is not safe as no destination specified, /var/tmp or /tmp will be used
-let diskupload = multer({ 
-  storage: diskStorage,
-});
-----
-
-=== Compliant solution
-
-https://www.npmjs.com/package/formidable[formidable] module:
-
-[source,javascript]
-----
-const Formidable = require('formidable');
-
-const form = new Formidable(); // Compliant
-form.uploadDir = "./uploads/";
-form.keepExtensions = false;
-----
-
-https://www.npmjs.com/package/multer[multer] (Express.js middleware) module:
-
-[source,javascript]
-----
-const multer = require('multer');
-
-let diskStorage = multer.diskStorage({  // Compliant
-  filename: (req, file, cb) => {
-    const buf = crypto.randomBytes(20);
-    cb(null, buf.toString('hex'))
-  },
-  destination: (req, file, cb) => {
-    cb(null, './uploads/')
-  }
-});
-
-let diskupload = multer({
-  storage: diskStorage,
-});
-----
-
-include::../see.adoc[]
 
 ifdef::env-github,rspecator-view[]
 
@@ -76,12 +24,22 @@ ifdef::env-github,rspecator-view[]
 == Implementation Specification
 (visible only on this page)
 
-include::../message.adoc[]
+=== Message
+
+Restrict [the extension|folder destination] of uploaded files.
 
 '''
 == Comments And Links
 (visible only on this page)
 
-include::../comments-and-links.adoc[]
+=== on 21 Jan 2021, 15:37:26 Pierre-Loup Tristant wrote:
+This rule is likely not implementable for C#.  ASP.NET Core is not providing
+any high level interface to help developper manage uploaded files.
+
+There is no temporary storage of uploaded file by default. The file stays in
+memory and it's up to the developper to chose the end location.
+
+Verifying file extention can be done in many different ways.
+
 
 endif::env-github,rspecator-view[]

rules/S2598/message.adoc

@@ -1,4 +0,0 @@
-=== Message
-
-Restrict [the extension|folder destination] of uploaded files.
-

rules/S2598/metadata.json

@@ -51,5 +51,9 @@
   "defaultQualityProfiles": [
     "Sonar way"
   ],
+  "educationPrinciples": [
+    "defense_in_depth",
+    "never_trust_user_input"
+  ],
   "quickfix": "unknown"
 }

rules/S2598/rationale.adoc

@@ -0,0 +1,8 @@
+If the file upload feature is implemented without proper folder restriction, it
+will result in an implicit trust violation within the server, as trusted files
+will be implicitly stored alongside third-party files that should be considered
+untrusted.
+
+This can allow an attacker to disrupt the security of an internal server
+process or the running application.
+

rules/S2598/rule.adoc

@@ -1,5 +0,0 @@
-== Why is this an issue?
-
-include::description.adoc[]
-
-include::see.adoc[]

rules/S2598/see.adoc

@@ -1,7 +0,0 @@
-== Resources
-
-* https://owasp.org/Top10/A04_2021-Insecure_Design/[OWASP Top 10 2021 Category A4] - Insecure Design
-* https://cwe.mitre.org/data/definitions/434[MITRE, CWE-434] - Unrestricted Upload of File with Dangerous Type
-* https://cwe.mitre.org/data/definitions/400[MITRE, CWE-400] - Uncontrolled Resource Consumption
-* https://owasp.org/www-community/vulnerabilities/Unrestricted_File_Upload[OWASP Unrestricted File Upload] - Unrestricted File Upload
-* https://www.sans.org/top25-software-errors/#cat1[SANS Top 25] - Insecure Interaction Between Components