Skip to content

Commit 71c279f

Browse files
MathiasVPMathiasVP
committed
Merge branch 'main' into use-range-analysis-in-buffer-write
2 parents 3f0bfe1 + 1a98079 commit 71c279f

File tree

189 files changed

+4197
-453
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

189 files changed

+4197
-453
lines changed

‎cpp/change-notes/2021-11-09-use-of-http.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lgtm,codescanning
2+
* A new query `cpp/non-https-url` has been added for C/C++. The query flags uses of `http` URLs that might be better replaced with `https`.

‎cpp/ql/src/Security/CWE/CWE-319/UseOfHttp.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
2+
void openUrl(char *url)
3+
{
4+
// ...
5+
}
6+
7+
openUrl("http://example.com"); // BAD
8+
9+
openUrl("https://example.com"); // GOOD: Opening a connection to a URL using HTTPS enforces SSL.

‎cpp/ql/src/Security/CWE/CWE-319/UseOfHttp.qhelp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
7+
<p>Constructing URLs with the HTTP protocol can lead to unsecured connections.</p>
8+
9+
</overview>
10+
<recommendation>
11+
12+
<p>When you construct a URL, ensure that you use an HTTPS URL rather than an HTTP URL. Then, any connections that are made using that URL are secure SSL connections.</p>
13+
14+
</recommendation>
15+
<example>
16+
17+
<p>The following example shows two ways of opening a connection using a URL. When the connection is
18+
opened using an HTTP URL rather than an HTTPS URL, the connection is unsecured. When the connection is opened using an HTTPS URL, the connection is a secure SSL connection.</p>
19+
20+
<sample src="UseOfHttp.cpp" />
21+
22+
</example>
23+
<references>
24+
25+
<li>
26+
OWASP:
27+
<a href="https://cheatsheetseries.owasp.org/cheatsheets/Transport_Layer_Protection_Cheat_Sheet.html">Transport Layer Protection Cheat Sheet</a>.
28+
</li>
29+
<li>
30+
OWASP Top 10:
31+
<a href="https://owasp.org/Top10/A08_2021-Software_and_Data_Integrity_Failures/">A08:2021 - Software and Data Integrity Failures</a>.
32+
</li>
33+
34+
</references>
35+
</qhelp>

‎cpp/ql/src/Security/CWE/CWE-319/UseOfHttp.ql

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/**
2+
* @name Failure to use HTTPS URLs
3+
* @description Non-HTTPS connections can be intercepted by third parties.
4+
* @kind path-problem
5+
* @problem.severity warning
6+
* @precision medium
7+
* @id cpp/non-https-url
8+
* @tags security
9+
* external/cwe/cwe-319
10+
*/
11+
12+
import cpp
13+
import semmle.code.cpp.dataflow.TaintTracking
14+
import DataFlow::PathGraph
15+
16+
/**
17+
* A string matching private host names of IPv4 and IPv6, which only matches
18+
* the host portion therefore checking for port is not necessary.
19+
* Several examples are localhost, reserved IPv4 IP addresses including
20+
* 127.0.0.1, 10.x.x.x, 172.16.x,x, 192.168.x,x, and reserved IPv6 addresses
21+
* including [0:0:0:0:0:0:0:1] and [::1]
22+
*/
23+
class PrivateHostName extends string {
24+
bindingset[this]
25+
PrivateHostName() {
26+
this.regexpMatch("(?i)localhost(?:[:/?#].*)?|127\\.0\\.0\\.1(?:[:/?#].*)?|10(?:\\.[0-9]+){3}(?:[:/?#].*)?|172\\.16(?:\\.[0-9]+){2}(?:[:/?#].*)?|192.168(?:\\.[0-9]+){2}(?:[:/?#].*)?|\\[?0:0:0:0:0:0:0:1\\]?(?:[:/?#].*)?|\\[?::1\\]?(?:[:/?#].*)?")
27+
}
28+
}
29+
30+
/**
31+
* A string containing an HTTP URL not in a private domain.
32+
*/
33+
class HttpStringLiteral extends StringLiteral {
34+
HttpStringLiteral() {
35+
exists(string s | this.getValue() = s |
36+
s = "http"
37+
or
38+
exists(string tail |
39+
tail = s.regexpCapture("http://(.*)", 1) and not tail instanceof PrivateHostName
40+
) and
41+
not TaintTracking::localExprTaint(any(StringLiteral p |
42+
p.getValue() instanceof PrivateHostName
43+
), this.getParent*())
44+
)
45+
}
46+
}
47+
48+
/**
49+
* Taint tracking configuration for HTTP connections.
50+
*/
51+
class HttpStringToUrlOpenConfig extends TaintTracking::Configuration {
52+
HttpStringToUrlOpenConfig() { this = "HttpStringToUrlOpenConfig" }
53+
54+
override predicate isSource(DataFlow::Node src) {
55+
// Sources are strings containing an HTTP URL not in a private domain.
56+
src.asExpr() instanceof HttpStringLiteral
57+
}
58+
59+
override predicate isSink(DataFlow::Node sink) {
60+
// Sinks can be anything that demonstrates the string is likely to be
61+
// accessed as a URL, for example using it in a network access. Some
62+
// URLs are only ever displayed or used for data processing.
63+
exists(FunctionCall fc |
64+
fc.getTarget().hasGlobalOrStdName(["system", "gethostbyname", "getaddrinfo"]) and
65+
sink.asExpr() = fc.getArgument(0)
66+
or
67+
fc.getTarget().hasGlobalOrStdName(["send", "URLDownloadToFile", "URLDownloadToCacheFile"]) and
68+
sink.asExpr() = fc.getArgument(1)
69+
or
70+
fc.getTarget().hasGlobalOrStdName(["curl_easy_setopt", "getnameinfo"]) and
71+
sink.asExpr() = fc.getArgument(2)
72+
or
73+
fc.getTarget().hasGlobalOrStdName(["ShellExecute", "ShellExecuteA", "ShellExecuteW"]) and
74+
sink.asExpr() = fc.getArgument(3)
75+
)
76+
}
77+
}
78+
79+
from
80+
HttpStringToUrlOpenConfig config, DataFlow::PathNode source, DataFlow::PathNode sink,
81+
HttpStringLiteral str
82+
where
83+
config.hasFlowPath(source, sink) and
84+
str = source.getNode().asExpr()
85+
select str, source, sink, "A URL may be constructed with the HTTP protocol."

‎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)