Skip to content

Commit bf9b8cf

Browse files
MathiasVPMathiasVP
authored
Merge pull request #6947 from ihsinme/ihsinme-patch-077
CPP: Add query for CWE-377 Insecure Temporary File
2 parents e0b876d + a044824 commit bf9b8cf

File tree

6 files changed

+220
-0
lines changed

6 files changed

+220
-0
lines changed

‎cpp/ql/src/experimental/Security/CWE/CWE-377/InsecureTemporaryFile.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
...
2+
fp = fopen("/tmp/name.tmp","w"); // BAD
3+
...
4+
char filename = tmpnam(NULL);
5+
fp = fopen(filename,"w"); // BAD
6+
...
7+
8+
strcat (filename, "/tmp/name.XXXXXX");
9+
fd = mkstemp(filename);
10+
if ( fd < 0 ) {
11+
return error;
12+
}
13+
fp = fdopen(fd,"w") // GOOD
14+
...

‎cpp/ql/src/experimental/Security/CWE/CWE-377/InsecureTemporaryFile.qhelp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>Working with a file, without checking its existence and its rights, as well as working with names that can be predicted, may not be safe. Requires the attention of developers.</p>
7+
8+
</overview>
9+
10+
<example>
11+
<p>The following example demonstrates erroneous and corrected work with file.</p>
12+
<sample src="InsecureTemporaryFile.cpp" />
13+
14+
</example>
15+
<references>
16+
17+
<li>
18+
CERT C Coding Standard:
19+
<a href="https://wiki.sei.cmu.edu/confluence/display/c/CON33-C.+Avoid+race+conditions+when+using+library+functions">CON33-C. Avoid race conditions when using library functions</a>.
20+
</li>
21+
22+
</references>
23+
</qhelp>

‎cpp/ql/src/experimental/Security/CWE/CWE-377/InsecureTemporaryFile.ql

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/**
2+
* @name Insecure generation of filenames.
3+
* @description Using a predictable filename when creating a temporary file can lead to an attacker-controlled input.
4+
* @kind problem
5+
* @id cpp/insecure-generation-of-filename
6+
* @problem.severity warning
7+
* @precision medium
8+
* @tags correctness
9+
* security
10+
* external/cwe/cwe-377
11+
*/
12+
13+
import cpp
14+
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
15+
16+
/** Holds for a function `f` that has an argument at index `apos` used to read the file. */
17+
predicate numberArgumentRead(Function f, int apos) {
18+
f.hasGlobalOrStdName("fgets") and apos = 2
19+
or
20+
f.hasGlobalOrStdName("fread") and apos = 3
21+
or
22+
f.hasGlobalOrStdName("read") and apos = 0
23+
or
24+
f.hasGlobalOrStdName("fscanf") and apos = 0
25+
}
26+
27+
/** Holds for a function `f` that has an argument at index `apos` used to write to file */
28+
predicate numberArgumentWrite(Function f, int apos) {
29+
f.hasGlobalOrStdName("fprintf") and apos = 0
30+
or
31+
f.hasGlobalOrStdName("fputs") and apos = 1
32+
or
33+
f.hasGlobalOrStdName("write") and apos = 0
34+
or
35+
f.hasGlobalOrStdName("fwrite") and apos = 3
36+
or
37+
f.hasGlobalOrStdName("fflush") and apos = 0
38+
}
39+
40+
from FunctionCall fc, string msg
41+
where
42+
// search for functions for generating a name, without a guarantee of the absence of a file during the period of work with it.
43+
(
44+
fc.getTarget().hasGlobalOrStdName("tmpnam") or
45+
fc.getTarget().hasGlobalOrStdName("tmpnam_s") or
46+
fc.getTarget().hasGlobalOrStdName("tmpnam_r")
47+
) and
48+
not exists(FunctionCall fctmp |
49+
(
50+
fctmp.getTarget().hasGlobalOrStdName("mktemp") or
51+
fctmp.getTarget().hasGlobalOrStdName("mkstemp") or
52+
fctmp.getTarget().hasGlobalOrStdName("mkstemps") or
53+
fctmp.getTarget().hasGlobalOrStdName("mkdtemp")
54+
) and
55+
(
56+
fc.getBasicBlock().getASuccessor*() = fctmp.getBasicBlock() or
57+
fctmp.getBasicBlock().getASuccessor*() = fc.getBasicBlock()
58+
)
59+
) and
60+
msg =
61+
"Finding the name of a file that does not exist does not mean that it will not be exist at the next operation."
62+
or
63+
// finding places to work with a file without setting permissions, but with predictable names.
64+
(
65+
fc.getTarget().hasGlobalOrStdName("fopen") or
66+
fc.getTarget().hasGlobalOrStdName("open")
67+
) and
68+
fc.getNumberOfArguments() = 2 and
69+
exists(FunctionCall fctmp, int i |
70+
numberArgumentWrite(fctmp.getTarget(), i) and
71+
globalValueNumber(fc) = globalValueNumber(fctmp.getArgument(i))
72+
) and
73+
not exists(FunctionCall fctmp, int i |
74+
numberArgumentRead(fctmp.getTarget(), i) and
75+
globalValueNumber(fc) = globalValueNumber(fctmp.getArgument(i))
76+
) and
77+
exists(FunctionCall fctmp |
78+
(
79+
fctmp.getTarget().hasGlobalOrStdName("strcat") or
80+
fctmp.getTarget().hasGlobalOrStdName("strcpy")
81+
) and
82+
globalValueNumber(fc.getArgument(0)) = globalValueNumber(fctmp.getAnArgument())
83+
or
84+
fctmp.getTarget().hasGlobalOrStdName("getenv") and
85+
globalValueNumber(fc.getArgument(0)) = globalValueNumber(fctmp)
86+
or
87+
(
88+
fctmp.getTarget().hasGlobalOrStdName("asprintf") or
89+
fctmp.getTarget().hasGlobalOrStdName("vasprintf") or
90+
fctmp.getTarget().hasGlobalOrStdName("xasprintf") or
91+
fctmp.getTarget().hasGlobalOrStdName("xvasprintf ")
92+
) and
93+
exists(Variable vrtmp |
94+
vrtmp = fc.getArgument(0).(VariableAccess).getTarget() and
95+
vrtmp = fctmp.getArgument(0).(AddressOfExpr).getAddressable().(Variable) and
96+
not vrtmp instanceof Field
97+
)
98+
) and
99+
not exists(FunctionCall fctmp |
100+
(
101+
fctmp.getTarget().hasGlobalOrStdName("umask") or
102+
fctmp.getTarget().hasGlobalOrStdName("fchmod") or
103+
fctmp.getTarget().hasGlobalOrStdName("chmod")
104+
) and
105+
(
106+
fc.getBasicBlock().getASuccessor*() = fctmp.getBasicBlock() or
107+
fctmp.getBasicBlock().getASuccessor*() = fc.getBasicBlock()
108+
)
109+
) and
110+
msg =
111+
"Creating a file for writing without evaluating its existence and setting permissions can be unsafe."
112+
select fc, msg

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-377/semmle/tests/InsecureTemporaryFile.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
| test.cpp:16:20:16:25 | call to tmpnam | Finding the name of a file that does not exist does not mean that it will not be exist at the next operation. |
2+
| test.cpp:42:8:42:12 | call to fopen | Creating a file for writing without evaluating its existence and setting permissions can be unsafe. |

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-377/semmle/tests/InsecureTemporaryFile.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE/CWE-377/InsecureTemporaryFile.ql

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-377/semmle/tests/test.cpp

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
typedef int FILE;
2+
#define NULL (0)
3+
FILE *fopen(char *filename, const char *mode);
4+
FILE *fdopen(int handle, char *mode);
5+
char * tmpnam(char * name);
6+
int mkstemp(char * name);
7+
char * strcat(char *str1, const char *str2);
8+
int umask(int pmode);
9+
int chmod(char * filename,int pmode);
10+
int fprintf(FILE *fp,const char *fmt, ...);
11+
int fclose(FILE *stream);
12+
13+
int funcTest1()
14+
{
15+
FILE *fp;
16+
char *filename = tmpnam(NULL); // BAD
17+
fp = fopen(filename,"w");
18+
fprintf(fp,"%s\n","data to file");
19+
fclose(fp);
20+
return 0;
21+
}
22+
23+
int funcTest2()
24+
{
25+
FILE *fp;
26+
int fd;
27+
char filename[80];
28+
strcat (filename, "/tmp/name.XXXXXX");
29+
fd = mkstemp(filename);
30+
if ( fd < 0 ) {
31+
return 1;
32+
}
33+
fp = fdopen(fd,"w"); // GOOD
34+
return 0;
35+
}
36+
37+
int funcTest3()
38+
{
39+
FILE *fp;
40+
char filename[80];
41+
strcat(filename, "/tmp/tmp.name");
42+
fp = fopen(filename,"w"); // BAD
43+
fprintf(fp,"%s\n","data to file");
44+
fclose(fp);
45+
return 0;
46+
}
47+
48+
int funcTest4()
49+
{
50+
FILE *fp;
51+
char filename[80];
52+
umask(0022);
53+
strcat(filename, "/tmp/tmp.name");
54+
fp = fopen(filename,"w"); // GOOD
55+
chmod(filename,0666);
56+
fprintf(fp,"%s\n","data to file");
57+
fclose(fp);
58+
return 0;
59+
}
60+
61+
int main(int argc, char *argv[])
62+
{
63+
funcTest1();
64+
funcTest2();
65+
funcTest3();
66+
funcTest4();
67+
return 0;
68+
}

0 commit comments

Comments
 (0)