Skip to content

Commit 70c643a

Browse files
mbaludambaluda
committed
Rule FIO45-C
1 parent 05f017c commit 70c643a

File tree

7 files changed

+325
-7
lines changed

7 files changed

+325
-7
lines changed

‎.vscode/tasks.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@
210210
"IO1",
211211
"IO2",
212212
"IO3",
213+
"IO4",
213214
"Includes",
214215
"Initialization",
215216
"IntegerConversion",

‎c/cert/src/rules/FIO45-C/ToctouRaceConditionsWhileAccessingFiles.md

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
# FIO45-C: Avoid TOCTOU race conditions while accessing files
2+
3+
This query implements the CERT-C rule FIO45-C:
4+
5+
> Avoid TOCTOU race conditions while accessing files
6+
7+
8+
## Description
9+
10+
A [TOCTOU](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-TOCTOU) (time-of-check, time-of-use) race condition is possible when two or more concurrent processes are operating on a shared file system \[[Seacord 2013b](https://wiki.sei.cmu.edu/confluence/display/c/AA.+Bibliography#AA.Bibliography-Seacord2013)\]. Typically, the first access is a check to verify some attribute of the file, followed by a call to use the file. An attacker can alter the file between the two accesses, or replace the file with a symbolic or hard link to a different file. These TOCTOU conditions can be exploited when a program performs two or more file operations on the same file name or path name.
11+
12+
A program that performs two or more file operations on a single file name or path name creates a race window between the two file operations. This race window comes from the assumption that the file name or path name refers to the same resource both times. If an attacker can modify the file, remove it, or replace it with a different file, then this assumption will not hold.
13+
14+
## Noncompliant Code Example
15+
16+
If an existing file is opened for writing with the `w` mode argument, the file's previous contents (if any) are destroyed. This noncompliant code example tries to prevent an existing file from being overwritten by first opening it for reading before opening it for writing. An attacker can exploit the race window between the two calls to `fopen()` to overwrite an existing file.
17+
18+
```cpp
19+
#include <stdio.h>
20+
21+
void open_some_file(const char *file) {
22+
FILE *f = fopen(file, "r");
23+
if (NULL != f) {
24+
/* File exists, handle error */
25+
} else {
26+
if (fclose(f) == EOF) {
27+
/* Handle error */
28+
}
29+
f = fopen(file, "w");
30+
if (NULL == f) {
31+
/* Handle error */
32+
}
33+
34+
/* Write to file */
35+
if (fclose(f) == EOF) {
36+
/* Handle error */
37+
}
38+
}
39+
}
40+
41+
```
42+
43+
## Compliant Solution
44+
45+
This compliant solution invokes `fopen()` at a single location and uses the `x` mode of `fopen()`, which was added in C11. This mode causes `fopen()` to fail if the file exists. This check and subsequent open is performed without creating a race window. The `x` mode provides exclusive access to the file only if the host environment provides this support.
46+
47+
```cpp
48+
#include <stdio.h>
49+
50+
void open_some_file(const char *file) {
51+
FILE *f = fopen(file, "wx");
52+
if (NULL == f) {
53+
/* Handle error */
54+
}
55+
/* Write to file */
56+
if (fclose(f) == EOF) {
57+
/* Handle error */
58+
}
59+
}
60+
```
61+
62+
## Compliant Solution (POSIX)
63+
64+
This compliant solution uses the `O_CREAT` and `O_EXCL` flags of POSIX's `open()` function. These flags cause `open()` to fail if the file exists.
65+
66+
```cpp
67+
#include <stdio.h>
68+
#include <unistd.h>
69+
#include <fcntl.h>
70+
71+
void open_some_file(const char *file) {
72+
int fd = open(file, O_CREAT | O_EXCL | O_WRONLY);
73+
if (-1 != fd) {
74+
FILE *f = fdopen(fd, "w");
75+
if (NULL != f) {
76+
/* Write to file */
77+
78+
if (fclose(f) == EOF) {
79+
/* Handle error */
80+
}
81+
} else {
82+
if (close(fd) == -1) {
83+
/* Handle error */
84+
}
85+
}
86+
}
87+
}
88+
```
89+
90+
## Exceptions
91+
92+
**FIO45-C-EX2:** Accessing a file name or path name multiple times is permitted if the file referenced resides in a secure directory. (For more information, see [FIO15-C. Ensure that file operations are performed in a secure directory](https://wiki.sei.cmu.edu/confluence/display/c/FIO15-C.+Ensure+that+file+operations+are+performed+in+a+secure+directory).)
93+
94+
**FIO45-C-EX3:** Accessing a file name or path name multiple times is permitted if the program can verify that every operation operates on the same file.
95+
96+
This POSIX code example verifies that each subsequent file access operates on the same file. In POSIX, every file can be uniquely identified by using its device and i-node attributes. This code example checks that a file name refers to a regular file (and not a directory, symbolic link, or other special file) by invoking `lstat()`. This call also retrieves its device and i-node. The file is subsequently opened. Finally, the program verifies that the file that was opened is the same one (matching device and i-nodes) as the file that was confirmed as a regular file.
97+
98+
An attacker can still exploit this code if they have the ability to delete the benign file and create the malicious file within the race window between lstat() and open(). It is possible that the OS kernel will reuse the same device and i-node for both files. This can be mitigated by making sure that the attacker lacks the permissions to delete the benign file.
99+
100+
```cpp
101+
#include <sys/stat.h>
102+
#include <fcntl.h>
103+
104+
int open_regular_file(char *filename, int flags) {
105+
struct stat lstat_info;
106+
struct stat fstat_info;
107+
int f;
108+
109+
if (lstat(filename, &lstat_info) == -1) {
110+
/* File does not exist, handle error */
111+
}
112+
113+
if (!S_ISREG(lstat_info.st_mode)) {
114+
/* File is not a regular file, handle error */
115+
}
116+
117+
if ((f = open(filename, flags)) == -1) {
118+
/* File has disappeared, handle error */
119+
}
120+
121+
if (fstat(f, &fstat_info) == -1) {
122+
/* Handle error */
123+
}
124+
125+
if (lstat_info.st_ino != fstat_info.st_ino ||
126+
lstat_info.st_dev != fstat_info.st_dev) {
127+
/* Open file is not the expected regular file, handle error */
128+
}
129+
130+
/* f is the expected regular open file */
131+
return f;
132+
}
133+
```
134+
135+
## Risk Assessment
136+
137+
[TOCTOU](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-TOCTOU) race conditions can result in [unexpected behavior](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-unexpectedbehavior), including privilege escalation.
138+
139+
<table> <tbody> <tr> <th> Rule </th> <th> Severity </th> <th> Likelihood </th> <th> Remediation Cost </th> <th> Priority </th> <th> Level </th> </tr> <tr> <td> FIO45-C </td> <td> High </td> <td> Probable </td> <td> High </td> <td> <strong>P6</strong> </td> <td> <strong>L2</strong> </td> </tr> </tbody> </table>
140+
141+
142+
## Automated Detection
143+
144+
<table> <tbody> <tr> <th> Tool </th> <th> Version </th> <th> Checker </th> <th> Description </th> </tr> <tr> <td> <a> CodeSonar </a> </td> <td> 7.0p0 </td> <td> <strong>IO.RACE</strong> </td> <td> File system race condition </td> </tr> <tr> <td> <a> Coverity </a> </td> <td> 2017.07 </td> <td> <strong>TOCTOU</strong> </td> <td> Implemented </td> </tr> <tr> <td> <a> Helix QAC </a> </td> <td> 2022.2 </td> <td> <strong>C4851, C4852, C4853</strong> <strong>C++4851, C++4852, C++4853</strong> </td> <td> </td> </tr> <tr> <td> <a> Klocwork </a> </td> <td> 2022.2 </td> <td> <strong>SV.TOCTOU.FILE_ACCESS</strong> </td> <td> </td> </tr> <tr> <td> <a> LDRA tool suite </a> </td> <td> 9.7.1 </td> <td> <strong>75 D</strong> </td> <td> Partially implemented </td> </tr> <tr> <td> <a> Parasoft C/C++test </a> </td> <td> 2022.1 </td> <td> <strong>CERT_C-FIO45-a</strong> </td> <td> Avoid race conditions while accessing files </td> </tr> <tr> <td> <a> Polyspace Bug Finder </a> </td> <td> R2022a </td> <td> <a> CERT C: Rule FIO45-C </a> </td> <td> Checks for file access between time of check and use (rule fully covered) </td> </tr> </tbody> </table>
145+
146+
147+
## Related Vulnerabilities
148+
149+
Search for [vulnerabilities](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-vulnerability) resulting from the violation of this rule on the [CERT website](https://www.kb.cert.org/vulnotes/bymetric?searchview&query=FIELD+KEYWORDS+contains+FIO45-C).
150+
151+
## Bibliography
152+
153+
<table> <tbody> <tr> <td> \[ <a> Seacord 2013b </a> \] </td> <td> Chapter 7, "Files" </td> </tr> </tbody> </table>
154+
155+
156+
## Implementation notes
157+
158+
None
159+
160+
## References
161+
162+
* CERT-C: [FIO45-C: Avoid TOCTOU race conditions while accessing files](https://wiki.sei.cmu.edu/confluence/display/c)

‎c/cert/src/rules/FIO45-C/ToctouRaceConditionsWhileAccessingFiles.ql

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/**
2+
* @id c/cert/toctou-race-conditions-while-accessing-files
3+
* @name FIO45-C: Avoid TOCTOU race conditions while accessing files
4+
* @description Avoid TOCTOU race conditions accessing files
5+
* @kind problem
6+
* @precision high
7+
* @problem.severity error
8+
* @tags external/cert/id/fio45-c
9+
* correctness
10+
* security
11+
* external/cert/obligation/rule
12+
*/
13+
14+
import cpp
15+
import codingstandards.c.cert
16+
import codingstandards.cpp.standardlibrary.FileAccess
17+
import codingstandards.cpp.ReadErrorsAndEOF
18+
import semmle.code.cpp.dataflow.DataFlow
19+
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
20+
21+
/**
22+
* A function call that opens a file as read-only
23+
* but does not read the content of the file.
24+
*/
25+
class EmptyFOpenCall extends FOpenCall {
26+
EmptyFOpenCall() {
27+
this.isReadOnlyMode() and
28+
// the FILE is only used as argument to close or in a NULL check
29+
not exists(Expr x |
30+
this != x and
31+
DataFlow::localExprFlow(this, x) and
32+
not closed(x) and
33+
exists(EQExpr eq |
34+
eq.getAnOperand() = x and eq.getAnOperand() = any(NULLMacro m).getAnInvocation().getExpr()
35+
)
36+
)
37+
}
38+
}
39+
40+
// The same file is opened multiple times in different modes
41+
from EmptyFOpenCall emptyFopen, FOpenCall fopen
42+
where
43+
not isExcluded(emptyFopen, IO4Package::toctouRaceConditionsWhileAccessingFilesQuery()) and
44+
not fopen.isReadOnlyMode() and
45+
globalValueNumber(emptyFopen.getFilenameExpr()) = globalValueNumber(fopen.getFilenameExpr())
46+
select emptyFopen,
47+
"This call is trying to prevent an exsisting file to be overwritten by $@. An attacker might be able to exploit the race window between the two calls.",
48+
fopen, "another call"

‎c/cert/test/rules/FIO45-C/ToctouRaceConditionsWhileAccessingFiles.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
| test.c:4:13:4:17 | call to fopen | This call is trying to prevent an exsisting file to be overwritten by $@. An attacker might be able to exploit the race window between the two calls. | test.c:11:9:11:13 | call to fopen | another call |

‎c/cert/test/rules/FIO45-C/test.c

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
#include <stdio.h>
2+
3+
void f1(const char *file) {
4+
FILE *f = fopen(file, "r"); // NON_COMPLIANT
5+
if (NULL != f) {
6+
/* File exists, handle error */
7+
} else {
8+
if (fclose(f) == EOF) {
9+
/* Handle error */
10+
}
11+
f = fopen(file, "w");
12+
if (NULL == f) {
13+
/* Handle error */
14+
}
15+
16+
/* Write to file */
17+
if (fclose(f) == EOF) {
18+
/* Handle error */
19+
}
20+
}
21+
}
22+
23+
void f2(const char *file) {
24+
FILE *f = fopen(file, "wx"); // COMPLIANT
25+
if (NULL == f) {
26+
/* Handle error */
27+
}
28+
/* Write to file */
29+
if (fclose(f) == EOF) {
30+
/* Handle error */
31+
}
32+
}
33+
34+
#include <fcntl.h>
35+
#include <unistd.h>
36+
37+
void f3(const char *file) {
38+
int fd = open(file, O_CREAT | O_EXCL | O_WRONLY);
39+
if (-1 != fd) {
40+
FILE *f = fdopen(fd, "w"); // COMPLIANT
41+
if (NULL != f) {
42+
/* Write to file */
43+
44+
if (fclose(f) == EOF) {
45+
/* Handle error */
46+
}
47+
} else {
48+
if (close(fd) == -1) {
49+
/* Handle error */
50+
}
51+
}
52+
}
53+
}
54+
55+
#include <sys/stat.h>
56+
57+
int f4(char *filename, int flags) {
58+
struct stat lstat_info;
59+
struct stat fstat_info;
60+
int f;
61+
62+
if (lstat(filename, &lstat_info) == -1) {
63+
/* File does not exist, handle error */
64+
}
65+
66+
if (!S_ISREG(lstat_info.st_mode)) {
67+
/* File is not a regular file, handle error */
68+
}
69+
70+
if ((f = open(filename, flags)) == -1) { // COMPLIANT
71+
/* File has disappeared, handle error */
72+
}
73+
74+
if (fstat(f, &fstat_info) == -1) {
75+
/* Handle error */
76+
}
77+
78+
if (lstat_info.st_ino != fstat_info.st_ino ||
79+
lstat_info.st_dev != fstat_info.st_dev) {
80+
/* Open file is not the expected regular file, handle error */
81+
}
82+
83+
/* f is the expected regular open file */
84+
return f;
85+
}

‎cpp/common/src/codingstandards/cpp/standardlibrary/FileAccess.qll

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,22 +56,43 @@ class Wint_t extends Type {
5656
}
5757

5858
/**
59-
* A function call that opens a file
59+
* A standard function call that opens a file
6060
*/
6161
class FOpenCall extends FunctionCall {
62-
FOpenCall() { this.getTarget().hasGlobalName(["fopen", "fopen_s", "freopen", "freopen_s"]) }
62+
FOpenCall() {
63+
this.getTarget().hasGlobalName(["fopen", "fdopen", "fopen_s", "freopen", "freopen_s", "open"])
64+
}
6365

6466
/** The expression corresponding to the accessed file */
65-
Expr getFilenameExpr() { result = this.getArgument(getNumberOfArguments() - 2) }
67+
Expr getFilenameExpr() {
68+
this.getTarget().hasGlobalName("open") and result = this.getArgument(0)
69+
or
70+
result = this.getArgument(getNumberOfArguments() - 2)
71+
}
6672

67-
Expr getMode() { result = this.getArgument(getNumberOfArguments() - 1) }
73+
Expr getMode() {
74+
this.getTarget().hasGlobalName("open") and result = this.getArgument(1)
75+
or
76+
result = this.getArgument(getNumberOfArguments() - 1)
77+
}
6878

6979
// make predicate
7080
predicate isReadMode() { this.getMode().getValue() = ["r", "r+", "w+", "a+"] }
7181

7282
predicate isWriteMode() { this.getMode().getValue() = ["w", "a", "r+", "w+", "a+"] }
7383

74-
predicate isReadOnlyMode() { this.isReadMode() and not this.isWriteMode() }
84+
predicate isReadOnlyMode() {
85+
this.isReadMode() and not this.isWriteMode()
86+
or
87+
exists(MacroInvocation mi |
88+
mi.getMacroName() = "O_RDONLY" and
89+
(
90+
this.getMode() = mi.getExpr()
91+
or
92+
this.getMode().(BitwiseOrExpr).getAnOperand*() = mi.getExpr()
93+
)
94+
)
95+
}
7596

7697
predicate isReadWriteMode() { this.isReadMode() and this.isWriteMode() }
7798
}

‎rule_packages/c/IO4.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
"description": "Avoid TOCTOU race conditions accessing files",
1010
"kind": "problem",
1111
"name": "Avoid TOCTOU race conditions while accessing files",
12-
"precision": "very-high",
12+
"precision": "high",
1313
"severity": "error",
1414
"short_name": "ToctouRaceConditionsWhileAccessingFiles",
1515
"tags": [
@@ -20,7 +20,7 @@
2020
],
2121
"title": "Avoid TOCTOU race conditions while accessing files",
2222
"implementation_scope": {
23-
"description": "None"
23+
"description": "The query is limited to one specific class of TOCTOU race conditions."
2424
}
2525
},
2626
"FIO47-C": {

0 commit comments

Comments
 (0)