Skip to content

Commit 838c1db

Browse files
ESS-ENNESS-ENN
committed
changes in file-access-before-action in cpp
1 parent 55859ed commit 838c1db

5 files changed

+400
-0
lines changed

‎rules/c/security/file-stat-before-action-c.yml

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
id: file-stat-before-action-c
2+
language: c
3+
severity: warning
4+
message: >-
5+
A check is done with `stat` and then the file is used. There is no
6+
guarantee that the status of the file has not changed since the call to
7+
`stat` which may allow attackers to bypass permission checks.
8+
note: >-
9+
[CWE-367]: Time-of-check Time-of-use (TOCTOU) Race Condition
10+
[REFERENCES]
11+
- https://wiki.sei.cmu.edu/confluence/display/c/FIO45-C.+Avoid+TOCTOU+race+conditions+while+accessing+files
12+
utils:
13+
match_fopen_identifier:
14+
kind: identifier
15+
regex: unlink|fopen|freopen|remove|rename|access|open|stat|lstat|unlink|mkdir|rmdir|chdir|folly::readFile|folly::writeFile|folly::writeFileAtomic|folly::writeFileAtomicNoThrow|folly::File
16+
all:
17+
- inside:
18+
kind: call_expression
19+
inside:
20+
stopBy: end
21+
kind: expression_statement
22+
inside:
23+
kind: compound_statement
24+
inside:
25+
kind: if_statement
26+
has:
27+
stopBy: end
28+
kind: call_expression
29+
all:
30+
- has:
31+
kind: identifier
32+
regex: ^(fstatat|_fstatat)$
33+
- has:
34+
stopBy: neighbor
35+
kind: argument_list
36+
all:
37+
- has:
38+
stopBy: neighbor
39+
kind: identifier
40+
- has:
41+
stopBy: neighbor
42+
kind: call_expression
43+
all:
44+
- has:
45+
stopBy: neighbor
46+
kind: field_expression
47+
- has:
48+
stopBy: neighbor
49+
kind: argument_list
50+
51+
match_fopen_identifier_2:
52+
kind: identifier
53+
regex: unlink|fopen|freopen|remove|rename|access|open|stat|lstat|unlink|mkdir|rmdir|chdir|folly::readFile|folly::writeFile|folly::writeFileAtomic|folly::writeFileAtomicNoThrow|folly::File
54+
all:
55+
- inside:
56+
kind: call_expression
57+
inside:
58+
stopBy: end
59+
kind: expression_statement
60+
inside:
61+
kind: compound_statement
62+
inside:
63+
kind: if_statement
64+
has:
65+
stopBy: end
66+
kind: call_expression
67+
all:
68+
- has:
69+
stopBy: neighbor
70+
kind: identifier
71+
regex: "^stat|_stat|lstat|_lstat$"
72+
- has:
73+
stopBy: neighbor
74+
kind: argument_list
75+
has:
76+
stopBy: neighbor
77+
kind: call_expression
78+
79+
rule:
80+
any:
81+
- matches: match_fopen_identifier
82+
- matches: match_fopen_identifier_2

‎rules/cpp/security/file-stat-before-action-cpp.yml

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
id: file-stat-before-action-cpp
2+
language: cpp
3+
severity: warning
4+
message: >-
5+
A check is done with `stat` and then the file is used. There is no
6+
guarantee that the status of the file has not changed since the call to
7+
`stat` which may allow attackers to bypass permission checks.
8+
note: >-
9+
[CWE-367]: Time-of-check Time-of-use (TOCTOU) Race Condition
10+
[REFERENCES]
11+
- https://wiki.sei.cmu.edu/confluence/display/c/FIO45-C.+Avoid+TOCTOU+race+conditions+while+accessing+files
12+
utils:
13+
match_fopen_identifier:
14+
kind: identifier
15+
regex: unlink|fopen|freopen|remove|rename|access|open|stat|lstat|unlink|mkdir|rmdir|chdir|folly::readFile|folly::writeFile|folly::writeFileAtomic|folly::writeFileAtomicNoThrow|folly::File
16+
all:
17+
- inside:
18+
kind: call_expression
19+
inside:
20+
stopBy: end
21+
kind: expression_statement
22+
inside:
23+
kind: compound_statement
24+
inside:
25+
kind: if_statement
26+
has:
27+
stopBy: end
28+
kind: call_expression
29+
all:
30+
- has:
31+
kind: identifier
32+
regex: ^(fstatat|_fstatat)$
33+
- has:
34+
stopBy: neighbor
35+
kind: argument_list
36+
all:
37+
- has:
38+
stopBy: neighbor
39+
kind: identifier
40+
- has:
41+
stopBy: neighbor
42+
kind: call_expression
43+
all:
44+
- has:
45+
stopBy: neighbor
46+
kind: field_expression
47+
- has:
48+
stopBy: neighbor
49+
kind: argument_list
50+
51+
match_fopen_identifier_2:
52+
kind: identifier
53+
regex: unlink|fopen|freopen|remove|rename|access|open|stat|lstat|unlink|mkdir|rmdir|chdir|folly::readFile|folly::writeFile|folly::writeFileAtomic|folly::writeFileAtomicNoThrow|folly::File
54+
all:
55+
- inside:
56+
kind: call_expression
57+
inside:
58+
stopBy: end
59+
kind: expression_statement
60+
inside:
61+
kind: compound_statement
62+
inside:
63+
kind: if_statement
64+
has:
65+
stopBy: end
66+
kind: call_expression
67+
all:
68+
- has:
69+
stopBy: neighbor
70+
kind: identifier
71+
regex: "^stat|_stat|lstat|_lstat$"
72+
- has:
73+
stopBy: neighbor
74+
kind: argument_list
75+
has:
76+
stopBy: neighbor
77+
kind: call_expression
78+
79+
rule:
80+
any:
81+
- matches: match_fopen_identifier
82+
- matches: match_fopen_identifier_2

‎tests/__snapshots__/file-stat-before-action-c-snapshot.yml

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
id: file-stat-before-action-c
2+
snapshots:
3+
? |
4+
if (stat(file.c_str(), &buf) == 0)
5+
{
6+
7+
// Open the file for reading
8+
// ruleid: file-stat-before-action
9+
fp = fopen(file.c_str(), "r");
10+
if (fp == NULL)
11+
{
12+
char message[2560];
13+
sprintf(message, "File '%s' Cound Not be Opened", file.c_str());
14+
// DISPLAY_MSG_ERROR( this, message, "GetFileContents", "System" );
15+
throw message;
16+
}
17+
18+
// Read the file
19+
MvString s, ss;
20+
while (fgets(data, sizeof(data), fp) != (char *)0)
21+
{
22+
s = data;
23+
s.trimBoth();
24+
if (s.compare(0, 5, "GROUP") == 0)
25+
{
26+
// size_t t = s.find_last_of( ":" );
27+
size_t t = s.find(":");
28+
if (t != string::npos)
29+
{
30+
ss = s.substr(t + 1).c_str();
31+
ss.trimBoth();
32+
ss = ss.substr(1, ss.length() - 3).c_str();
33+
group_list.push_back(ss);
34+
}
35+
}
36+
}
37+
38+
// Close the file
39+
fclose(fp);
40+
}
41+
: labels:
42+
- source: fopen
43+
style: primary
44+
start: 123
45+
end: 128
46+
- source: stat
47+
style: secondary
48+
start: 4
49+
end: 8
50+
- source: file.c_str()
51+
style: secondary
52+
start: 9
53+
end: 21
54+
- source: (file.c_str(), &buf)
55+
style: secondary
56+
start: 8
57+
end: 28
58+
- source: stat(file.c_str(), &buf)
59+
style: secondary
60+
start: 4
61+
end: 28
62+
- source: |-
63+
if (stat(file.c_str(), &buf) == 0)
64+
{
65+
66+
// Open the file for reading
67+
// ruleid: file-stat-before-action
68+
fp = fopen(file.c_str(), "r");
69+
if (fp == NULL)
70+
{
71+
char message[2560];
72+
sprintf(message, "File '%s' Cound Not be Opened", file.c_str());
73+
// DISPLAY_MSG_ERROR( this, message, "GetFileContents", "System" );
74+
throw message;
75+
}
76+
77+
// Read the file
78+
MvString s, ss;
79+
while (fgets(data, sizeof(data), fp) != (char *)0)
80+
{
81+
s = data;
82+
s.trimBoth();
83+
if (s.compare(0, 5, "GROUP") == 0)
84+
{
85+
// size_t t = s.find_last_of( ":" );
86+
size_t t = s.find(":");
87+
if (t != string::npos)
88+
{
89+
ss = s.substr(t + 1).c_str();
90+
ss.trimBoth();
91+
ss = ss.substr(1, ss.length() - 3).c_str();
92+
group_list.push_back(ss);
93+
}
94+
}
95+
}
96+
97+
// Close the file
98+
fclose(fp);
99+
}
100+
style: secondary
101+
start: 0
102+
end: 989
103+
- source: |-
104+
{
105+
106+
// Open the file for reading
107+
// ruleid: file-stat-before-action
108+
fp = fopen(file.c_str(), "r");
109+
if (fp == NULL)
110+
{
111+
char message[2560];
112+
sprintf(message, "File '%s' Cound Not be Opened", file.c_str());
113+
// DISPLAY_MSG_ERROR( this, message, "GetFileContents", "System" );
114+
throw message;
115+
}
116+
117+
// Read the file
118+
MvString s, ss;
119+
while (fgets(data, sizeof(data), fp) != (char *)0)
120+
{
121+
s = data;
122+
s.trimBoth();
123+
if (s.compare(0, 5, "GROUP") == 0)
124+
{
125+
// size_t t = s.find_last_of( ":" );
126+
size_t t = s.find(":");
127+
if (t != string::npos)
128+
{
129+
ss = s.substr(t + 1).c_str();
130+
ss.trimBoth();
131+
ss = ss.substr(1, ss.length() - 3).c_str();
132+
group_list.push_back(ss);
133+
}
134+
}
135+
}
136+
137+
// Close the file
138+
fclose(fp);
139+
}
140+
style: secondary
141+
start: 36
142+
end: 989
143+
- source: fp = fopen(file.c_str(), "r");
144+
style: secondary
145+
start: 118
146+
end: 148
147+
- source: fopen(file.c_str(), "r")
148+
style: secondary
149+
start: 123
150+
end: 147

‎tests/c/file-stat-before-action-c-test.yml

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
id: file-stat-before-action-c
2+
valid:
3+
- |
4+
5+
invalid:
6+
- |
7+
if (stat(file.c_str(), &buf) == 0)
8+
{
9+
10+
// Open the file for reading
11+
// ruleid: file-stat-before-action
12+
fp = fopen(file.c_str(), "r");
13+
if (fp == NULL)
14+
{
15+
char message[2560];
16+
sprintf(message, "File '%s' Cound Not be Opened", file.c_str());
17+
// DISPLAY_MSG_ERROR( this, message, "GetFileContents", "System" );
18+
throw message;
19+
}
20+
21+
// Read the file
22+
MvString s, ss;
23+
while (fgets(data, sizeof(data), fp) != (char *)0)
24+
{
25+
s = data;
26+
s.trimBoth();
27+
if (s.compare(0, 5, "GROUP") == 0)
28+
{
29+
// size_t t = s.find_last_of( ":" );
30+
size_t t = s.find(":");
31+
if (t != string::npos)
32+
{
33+
ss = s.substr(t + 1).c_str();
34+
ss.trimBoth();
35+
ss = ss.substr(1, ss.length() - 3).c_str();
36+
group_list.push_back(ss);
37+
}
38+
}
39+
}
40+
41+
// Close the file
42+
fclose(fp);
43+
}

0 commit comments

Comments
 (0)