Modify S5847: Change text to LayC format (APPSEC-1214) (#3316)

rules/S5847/cfamily/rule.adoc

@@ -1,69 +1,69 @@
+include::../common/description.adoc[]
+
 == Why is this an issue?
 
-include::../description.adoc[]
+include::../common/rationale.adoc[]
 
-=== Noncompliant code example
+=== What is the potential impact?
 
-A "check function" (for instance ``++access++``, ``++stat++`` ... in this case ``++access++`` to verify the existence of a file) is used, followed by a "use function" (``++open++``, ``++fopen++`` ...) to write data inside a non existing file. These two consecutive calls create a TOCTOU race condition:
+include::../common/impact/rationale.adoc[]
 
-[source,cpp]
-----
-#include <stdio.h>
+include::../common/impact/code_execution.adoc[]
 
-void fopen_with_toctou(const char *file) {
-  if (access(file, F_OK) == -1 && errno == ENOENT) {
-    // the file doesn't exist
-    // it is now created in order to write some data inside
-    FILE *f = fopen(file, "w"); // Noncompliant: a race condition window exist from access() call to fopen() call calls
-    if (NULL == f) {
-      /* Handle error */
-    }
-
-    if (fclose(f) == EOF) {
-      /* Handle error */
-    }
-  }
-}
-----
+include::../common/impact/privilege_escalation.adoc[]
 
-=== Compliant solution
+include::../common/impact/dos.adoc[]
 
-If the file already exists on the disk, ``++fopen++`` with ``++x++`` mode will fail:
+== How to fix it
 
-[source,cpp]
-----
-#include <stdio.h>
+include::../common/how-to-fix/rationale.adoc[]
 
-void open_without_toctou(const char *file) {
-  FILE *f = fopen(file, "wx"); // Compliant
-  if (NULL == f) {
-    /* Handle error */
-  }
-  /* Write to file */
-  if (fclose(f) == EOF) {
-    /* Handle error */
+=== Code examples
+
+==== Noncompliant code example
+
+The following code sample is susceptible to a race condition attack because it
+checks a file exists strictly before it opens it for writing.
+
+[source,C++,diff-id=1,diff-type=noncompliant]
+----
+FILE *fopen_if_not_exists(const char *file) {
+  if (access(file, F_OK) == -1 && errno == ENOENT) {
+    FILE *f = fopen(file, "w"); // Noncompliant
+    
+    return f;
   }
+
+  return nullptr;
 }
 ----
 
-A more generic solution is to use "file descriptors":
+==== Compliant solution
 
-[source,cpp]
+[source,C++,diff-id=1,diff-type=compliant]
 ----
-void open_without_toctou(const char *file) {
-  int fd = open(file, O_CREAT | O_EXCL | O_WRONLY);
-  if (-1 != fd) {
-    FILE *f = fdopen(fd, "w");  // Compliant
-  }
+FILE *fopen_if_not_exists(const char *file) {
+  FILE *f = fopen(file, "wx");
+  return f;
 }
 ----
 
+=== How does this work?
+
+Here, the compliant code example uses an atomic operation to open the file and
+check for its existence beforehand.
+
 == Resources
 
-* https://owasp.org/Top10/A01_2021-Broken_Access_Control/[OWASP Top 10 2021 Category A1] - Broken Access Control
-* 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/367[MITRE, CWE-367] - Time-of-check Time-of-use (TOCTOU) Race Condition
-* https://wiki.sei.cmu.edu/confluence/display/c/FIO45-C.+Avoid+TOCTOU+race+conditions+while+accessing+files[CERT, FIO45-C.] - Avoid TOCTOU race conditions while accessing files
+=== Documentation
+
+* Carnegie Mellon University Software Engineering Institure - https://wiki.sei.cmu.edu/confluence/display/c/FIO45-C.+Avoid+TOCTOU+race+conditions+while+accessing+files[FIO45-C. - Avoid TOCTOU race conditions while accessing files]
+
+=== Standards
+
+* OWASP - https://owasp.org/Top10/A01_2021-Broken_Access_Control/[Top 10 2021 Category A1 - Broken Access Control]
+* OWASP - https://owasp.org/www-project-top-ten/2017/A5_2017-Broken_Access_Control[Top 10 2017 Category A5 - Broken Access Control]
+* CWE - https://cwe.mitre.org/data/definitions/367[CWE-367 - Time-of-check Time-of-use (TOCTOU) Race Condition]
 
 ifdef::env-github,rspecator-view[]
 
@@ -80,3 +80,4 @@ include::../message.adoc[]
 include::../comments-and-links.adoc[]
 
 endif::env-github,rspecator-view[]
+

rules/S5847/common/description.adoc

@@ -0,0 +1,6 @@
+When an application manipulates files, "Time-Of-Check to Time-Of-Use" can occur
+when a file-checking operation is disconnected from the actual operation it is
+bound to.
+
+For example, such a vulnerability occurs when a file existence check is
+performed strictly before a file creation operation.

rules/S5847/common/how-to-fix/rationale.adoc

@@ -0,0 +1,20 @@
+To prevent TOCTOU race condition issues, best practices recommend relying on
+file operations that can perform the necessary preliminary checks atomically.
+For example, file opening functions usually accept a parameter to check the file
+exists and return an error depending on the result. This check is atomic and is
+not susceptible to race conditions. 
+
+When this is not possible, it might be possible to open a file directly, and to
+keep a reference to it for later use. If the conditions are set for the
+subsequent operations, the application can continue with its processing and use
+the open file pointer to read or write to the file.  In the opposite case, an
+error might be raised that will need to be properly handled.
+
+To finish, for most complex operations, the application can create a dedicated
+working directory and set tight permissions on it. This needs to be performed
+atomically to prevent further race conditions. All subsequent sensitive file
+operations can then be performed in this dedicated directory.
+
+Note that this last solution is imperfect and is still susceptible to race
+condition attacks from privileged users and the application itself. It should be
+used when no other countermeasure is acceptable.

rules/S5847/common/impact/code_execution.adoc

@@ -0,0 +1,11 @@
+==== Arbitrary code execution
+
+Executable or script file integrity and authenticity checks can be bypassed when
+exploiting a TOCTOU vulnerability. In such a scenario, attackers would change
+an executable file content between when its integrity is checked and when the
+application executes it.
+
+This attack would allow attackers to trick the application into executing
+malicious, arbitrary code. They would then be granted the same privilege levels
+as the application itself, which can be particularly severe when it runs with
+administration privileges.

rules/S5847/common/impact/dos.adoc

@@ -0,0 +1,10 @@
+==== Denial of service
+
+When the application expects some file properties to be set at the operation
+time, it will often face unexpected errors when those properties have actually
+changed. This might be the case when writing to a file where newly set
+permissions forbid that operation or when reading from a deleted file.
+
+When such errors are faced, the application might unexpectedly stop, which can
+affect its availability. Depending on the application and hosting architectures,
+the interruption can be temporary or permanent, partial or complete.

rules/S5847/common/impact/privilege_escalation.adoc

@@ -0,0 +1,12 @@
+==== Privileges escalation
+
+When the attacker is a local user on the same server as the running application,
+the same attack is possible with extended probability. In such a case, attackers
+can exploit the reading and writing to configuration files, the creation of
+local network resources, or the use of temporary files to achieve the same code
+execution purpose.
+
+However, in that case, the attack is only meaningful when the application is
+running with high or otherwise interesting privileges. Attackers exploiting a
+TOCTOU vulnerability that way would achieve horizontal or vertical privilege
+escalation.

rules/S5847/common/impact/rationale.adoc

@@ -0,0 +1,6 @@
+The impact of a successfully exploited race condition is dependent on the
+business logic of the vulnerable application. The consequences will vary
+depending on what check is performed and how the file is used.
+
+In general, attackers use such attacks to escalate privileges, execute arbitrary
+code, or perform a denial of service.

rules/S5847/common/rationale.adoc

@@ -0,0 +1,9 @@
+Race conditions can happen when file operations and their associated pre-checks
+are disconnected. Indeed, while the application assumes that the checked file
+property will not change before performing the operation, there is a chance that
+changes are applied to the file.
+
+Especially, a concurrent process, which an attacker can control, could modify a
+file right after a check is performed and before the actual use. This file can
+be deleted, created, altered, or see its permissions changed depending on the
+use case.