Skip to content

Commit cf56597

Browse files
ihsinmeihsinme
authored
Merge pull request #1 from github/main
update fork
2 parents 6966453 + 1068ede commit cf56597

File tree

518 files changed

+78492
-12063
lines changed

Some content is hidden

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

518 files changed

+78492
-12063
lines changed

‎.gitignore‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
# Byte-compiled python files
1818
*.pyc
1919

20+
# python virtual environment folder
21+
.venv/
22+
2023
# It's useful (though not required) to be able to unpack codeql in the ql checkout itself
2124
/codeql/
2225

‎cpp/ql/src/Security/CWE/CWE-089/SqlTainted.ql‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ class Configuration extends TaintTrackingConfiguration {
2727
override predicate isSink(Element tainted) {
2828
exists(SQLLikeFunction runSql | runSql.outermostWrapperFunctionCall(tainted, _))
2929
}
30+
31+
override predicate isBarrier(Expr e) {
32+
super.isBarrier(e) or e.getUnspecifiedType() instanceof IntegralType
33+
}
3034
}
3135

3236
from

‎cpp/ql/src/experimental/Security/CWE/CWE-14/CompilerRemovalOfCodeToClearBuffers.c‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// BAD: the memset call will probably be removed.
2+
void getPassword(void) {
3+
char pwd[64];
4+
if (GetPassword(pwd, sizeof(pwd))) {
5+
/* Checking of password, secure operations, etc. */
6+
}
7+
memset(pwd, 0, sizeof(pwd));
8+
}
9+
// GOOD: in this case the memset will not be removed.
10+
void getPassword(void) {
11+
char pwd[64];
12+
13+
if (retrievePassword(pwd, sizeof(pwd))) {
14+
/* Checking of password, secure operations, etc. */
15+
}
16+
memset_s(pwd, 0, sizeof(pwd));
17+
}
18+
// GOOD: in this case the memset will not be removed.
19+
void getPassword(void) {
20+
char pwd[64];
21+
if (retrievePassword(pwd, sizeof(pwd))) {
22+
/* Checking of password, secure operations, etc. */
23+
}
24+
SecureZeroMemory(pwd, sizeof(pwd));
25+
}
26+
// GOOD: in this case the memset will not be removed.
27+
void getPassword(void) {
28+
char pwd[64];
29+
if (retrievePassword(pwd, sizeof(pwd))) {
30+
/* Checking of password, secure operations, etc. */
31+
}
32+
#pragma optimize("", off)
33+
memset(pwd, 0, sizeof(pwd));
34+
#pragma optimize("", on)
35+
}

‎cpp/ql/src/experimental/Security/CWE/CWE-14/CompilerRemovalOfCodeToClearBuffers.qhelp‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>Compiler optimization will exclude the cleaning of private information.
7+
Using the <code>memset</code> function to clear private data in a variable that has no subsequent use is potentially dangerous, since the compiler can remove the call.
8+
For some compilers, optimization is also possible when using calls to free memory after the <code>memset</code> function.</p>
9+
10+
<p>It is possible to miss detection of vulnerabilities if used to clear fields of structures or parts of a buffer.</p>
11+
12+
</overview>
13+
<recommendation>
14+
15+
<p>We recommend to use the <code>RtlSecureZeroMemory</code> or <code>memset_s</code> functions, or compilation flags that exclude optimization of <code>memset</code> calls (e.g. -fno-builtin-memset).</p>
16+
17+
</recommendation>
18+
<example>
19+
<p>The following example demonstrates an erroneous and corrected use of the <code>memset</code> function.</p>
20+
<sample src="CompilerRemovalOfCodeToClearBuffers.c" />
21+
22+
</example>
23+
<references>
24+
25+
<li>
26+
CERT C Coding Standard:
27+
<a href="https://wiki.sei.cmu.edu/confluence/display/c/MSC06-C.+Beware+of+compiler+optimizations">MSC06-C. Beware of compiler optimizations</a>.
28+
</li>
29+
30+
</references>
31+
</qhelp>

‎cpp/ql/src/experimental/Security/CWE/CWE-14/CompilerRemovalOfCodeToClearBuffers.ql‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
/**
2+
* @name Compiler Removal Of Code To Clear Buffers
3+
* @description Using <code>memset</code> the function to clear private data in a variable that has no subsequent use
4+
* is potentially dangerous because the compiler can remove the call.
5+
* @kind problem
6+
* @id cpp/compiler-removal-of-code-to-clear-buffers
7+
* @problem.severity warning
8+
* @precision medium
9+
* @tags security
10+
* external/cwe/cwe-14
11+
*/
12+
13+
import cpp
14+
import semmle.code.cpp.dataflow.DataFlow
15+
import semmle.code.cpp.dataflow.StackAddress
16+
17+
/**
18+
* A call to `memset` of the form `memset(ptr, value, num)`, for some local variable `ptr`.
19+
*/
20+
class CompilerRemovaMemset extends FunctionCall {
21+
CompilerRemovaMemset() {
22+
this.getTarget().hasGlobalOrStdName("memset") and
23+
exists(DataFlow::Node source, DataFlow::Node sink, LocalVariable isv, Expr exp |
24+
DataFlow::localFlow(source, sink) and
25+
this.getArgument(0) = isv.getAnAccess() and
26+
(
27+
source.asExpr() = exp
28+
or
29+
// handle the case where exp is defined by an address being passed into some function.
30+
source.asDefiningArgument() = exp
31+
) and
32+
exp.getLocation().getEndLine() < this.getArgument(0).getLocation().getStartLine() and
33+
sink.asExpr() = this.getArgument(0)
34+
)
35+
}
36+
37+
predicate isExistsAllocForThisVariable() {
38+
exists(AllocationExpr alloc, Variable v |
39+
alloc = v.getAnAssignedValue() and
40+
this.getArgument(0) = v.getAnAccess() and
41+
alloc.getASuccessor+() = this
42+
)
43+
or
44+
not stackPointerFlowsToUse(this.getArgument(0), _, _, _)
45+
}
46+
47+
predicate isExistsFreeForThisVariable() {
48+
exists(DeallocationExpr free, Variable v |
49+
this.getArgument(0) = v.getAnAccess() and
50+
free.getFreedExpr() = v.getAnAccess() and
51+
this.getASuccessor+() = free
52+
)
53+
}
54+
55+
predicate isExistsCallWithThisVariableExcludingDeallocationCalls() {
56+
exists(FunctionCall fc, Variable v |
57+
not fc instanceof DeallocationExpr and
58+
this.getArgument(0) = v.getAnAccess() and
59+
fc.getAnArgument() = v.getAnAccess() and
60+
this.getASuccessor+() = fc
61+
)
62+
}
63+
64+
predicate isVariableUseAfterMemsetExcludingCalls() {
65+
exists(DataFlow::Node source, DataFlow::Node sink, LocalVariable isv, Expr exp |
66+
DataFlow::localFlow(source, sink) and
67+
this.getArgument(0) = isv.getAnAccess() and
68+
source.asExpr() = isv.getAnAccess() and
69+
exp.getLocation().getStartLine() > this.getArgument(2).getLocation().getEndLine() and
70+
not exp.getParent() instanceof FunctionCall and
71+
sink.asExpr() = exp
72+
)
73+
}
74+
75+
predicate isVariableUseBoundWithArgumentFunction() {
76+
exists(DataFlow::Node source, DataFlow::Node sink, LocalVariable isv, Parameter p, Expr exp |
77+
DataFlow::localFlow(source, sink) and
78+
this.getArgument(0) = isv.getAnAccess() and
79+
this.getEnclosingFunction().getAParameter() = p and
80+
exp.getAChild*() = p.getAnAccess() and
81+
source.asExpr() = exp and
82+
sink.asExpr() = isv.getAnAccess()
83+
)
84+
}
85+
86+
predicate isVariableUseBoundWithGlobalVariable() {
87+
exists(
88+
DataFlow::Node source, DataFlow::Node sink, LocalVariable isv, GlobalVariable gv, Expr exp
89+
|
90+
DataFlow::localFlow(source, sink) and
91+
this.getArgument(0) = isv.getAnAccess() and
92+
exp.getAChild*() = gv.getAnAccess() and
93+
source.asExpr() = exp and
94+
sink.asExpr() = isv.getAnAccess()
95+
)
96+
}
97+
98+
predicate isExistsCompilationFlagsBlockingRemoval() {
99+
exists(Compilation c |
100+
c.getAFileCompiled() = this.getFile() and
101+
c.getAnArgument() = "-fno-builtin-memset"
102+
)
103+
}
104+
105+
predicate isUseVCCompilation() {
106+
exists(Compilation c |
107+
c.getAFileCompiled() = this.getFile() and
108+
(
109+
c.getArgument(2).matches("%gcc%") or
110+
c.getArgument(2).matches("%g++%") or
111+
c.getArgument(2).matches("%clang%") or
112+
c.getArgument(2) = "--force-recompute"
113+
)
114+
)
115+
}
116+
}
117+
118+
from CompilerRemovaMemset fc
119+
where
120+
not (fc.isExistsAllocForThisVariable() and not fc.isExistsFreeForThisVariable()) and
121+
not (fc.isExistsFreeForThisVariable() and not fc.isUseVCCompilation()) and
122+
not fc.isVariableUseAfterMemsetExcludingCalls() and
123+
not fc.isExistsCallWithThisVariableExcludingDeallocationCalls() and
124+
not fc.isVariableUseBoundWithArgumentFunction() and
125+
not fc.isVariableUseBoundWithGlobalVariable() and
126+
not fc.isExistsCompilationFlagsBlockingRemoval()
127+
select fc.getArgument(0), "This variable will not be cleared."

‎cpp/ql/src/experimental/Security/CWE/CWE-359/PrivateCleartextWrite.ql‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,6 @@ import DataFlow::PathGraph
1616

1717
from WriteConfig b, DataFlow::PathNode source, DataFlow::PathNode sink
1818
where b.hasFlowPath(source, sink)
19-
select sink.getNode(),
20-
"This write into the external location '" + sink + "' may contain unencrypted data from $@",
21-
source, "this source."
19+
select sink.getNode(), source, sink,
20+
"This write into the external location '" + sink.getNode() +
21+
"' may contain unencrypted data from $@", source, "this source."

‎cpp/ql/src/experimental/Security/CWE/CWE-401/MemoryLeakOnFailedCallToRealloc.ql‎

Lines changed: 24 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,20 @@
1212
*/
1313

1414
import cpp
15+
import semmle.code.cpp.controlflow.Guards
16+
17+
/**
18+
* A function call that potentially does not return (such as `exit`).
19+
*/
20+
class CallMayNotReturn extends FunctionCall {
21+
CallMayNotReturn() {
22+
// call that is known to not return
23+
not exists(this.(ControlFlowNode).getASuccessor())
24+
or
25+
// call to another function that may not return
26+
exists(CallMayNotReturn exit | getTarget() = exit.getEnclosingFunction())
27+
}
28+
}
1529

1630
/**
1731
* A call to `realloc` of the form `v = realloc(v, size)`, for some variable `v`.
@@ -30,40 +44,19 @@ class ReallocCallLeak extends FunctionCall {
3044
)
3145
}
3246

33-
predicate isExistsIfWithExitCall() {
34-
exists(IfStmt ifc |
35-
this.getArgument(0) = v.getAnAccess() and
36-
ifc.getCondition().getAChild*() = v.getAnAccess() and
37-
ifc.getEnclosingFunction() = this.getEnclosingFunction() and
38-
ifc.getLocation().getStartLine() >= this.getArgument(0).getLocation().getStartLine() and
39-
exists(FunctionCall fc |
40-
fc.getTarget().hasName("exit") and
41-
fc.getEnclosingFunction() = this.getEnclosingFunction() and
42-
(ifc.getThen().getAChild*() = fc or ifc.getElse().getAChild*() = fc)
43-
)
44-
or
45-
exists(FunctionCall fc, FunctionCall ftmp1, FunctionCall ftmp2 |
46-
ftmp1.getTarget().hasName("exit") and
47-
ftmp2.(ControlFlowNode).getASuccessor*() = ftmp1 and
48-
fc = ftmp2.getEnclosingFunction().getACallToThisFunction() and
49-
fc.getEnclosingFunction() = this.getEnclosingFunction() and
50-
(ifc.getThen().getAChild*() = fc or ifc.getElse().getAChild*() = fc)
51-
)
52-
)
53-
}
54-
55-
predicate isExistsAssertWithArgumentCall() {
56-
exists(FunctionCall fc |
57-
fc.getTarget().hasName("__assert_fail") and
58-
this.getEnclosingFunction() = fc.getEnclosingFunction() and
59-
fc.getLocation().getStartLine() > this.getArgument(0).getLocation().getEndLine() and
60-
fc.getArgument(0).toString().matches("%" + this.getArgument(0).toString() + "%")
47+
/**
48+
* Holds if failure of this allocation may be handled by termination, for
49+
* example a call to `exit()`.
50+
*/
51+
predicate mayHandleByTermination() {
52+
exists(GuardCondition guard, CallMayNotReturn exit |
53+
this.(ControlFlowNode).getASuccessor*() = guard and
54+
guard.getAChild*() = v.getAnAccess() and
55+
guard.controls(exit.getBasicBlock(), _)
6156
)
6257
}
6358
}
6459

6560
from ReallocCallLeak rcl
66-
where
67-
not rcl.isExistsIfWithExitCall() and
68-
not rcl.isExistsAssertWithArgumentCall()
61+
where not rcl.mayHandleByTermination()
6962
select rcl, "possible loss of original pointer on unsuccessful call realloc"

‎cpp/ql/src/experimental/Security/CWE/CWE-788/AccessOfMemoryLocationAfterEndOfBufferUsingStrncat.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
2+
strncat(dest, source, sizeof(dest) - strlen(dest)); // BAD: writes a zero byte past the `dest` buffer.
3+
4+
strncat(dest, source, sizeof(dest) - strlen(dest) -1); // GOOD: Reserves space for the zero byte.

‎cpp/ql/src/experimental/Security/CWE/CWE-788/AccessOfMemoryLocationAfterEndOfBufferUsingStrncat.qhelp‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>The standard library function <code>strncat(dest, source, count)</code> appends the <code>source</code> string to the <code>dest</code> string. <code>count</code> specifies the maximum number of characters to append and must be less than the remaining space in the target buffer. Calls of the form <code> strncat (dest, source, sizeof (dest) - strlen (dest)) </code> set the third argument to one more than possible. So when the <code>dest</code> is full, the expression <code> sizeof (dest) - strlen (dest) </code> will be equal to one, and not zero as the programmer might think. Making a call of this type may result in a zero byte being written just outside the <code>dest</code> buffer.</p>
7+
8+
9+
</overview>
10+
<recommendation>
11+
12+
<p>We recommend subtracting one from the third argument. For example, replace <code>strncat(dest, source, sizeof(dest)-strlen(dest))</code> with <code>strncat(dest, source, sizeof(dest)-strlen(dest)-1)</code>.</p>
13+
14+
</recommendation>
15+
<example>
16+
<p>The following example demonstrates an erroneous and corrected use of the <code>strncat</code> function.</p>
17+
<sample src="AccessOfMemoryLocationAfterEndOfBufferUsingStrncat.c" />
18+
19+
</example>
20+
<references>
21+
22+
<li>
23+
CERT C Coding Standard:
24+
<a href="https://wiki.sei.cmu.edu/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator">STR31-C. Guarantee that storage for strings has sufficient space for character data and the null terminator</a>.
25+
</li>
26+
<li>
27+
CERT C Coding Standard:
28+
<a href="https://wiki.sei.cmu.edu/confluence/display/c/ARR30-C.+Do+not+form+or+use+out-of-bounds+pointers+or+array+subscripts">ARR30-C. Do not form or use out-of-bounds pointers or array subscripts</a>.
29+
</li>
30+
31+
</references>
32+
</qhelp>

0 commit comments

Comments
 (0)