Skip to content

Commit b185a33

Browse files
authored
Add files via upload
1 parent b28444b commit b185a33

File tree

3 files changed

+187
-0
lines changed

3 files changed

+187
-0
lines changed
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 optimized.
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 optimized.
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 optimized.
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 optimized.
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+
}
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 memset function to clear private data as a final expression when working with a variable is potentially dangerous, since the compiler can optimize this call.
8+
For some compilers, optimization is also possible when using calls to free memory after the <code>memset</codee> 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 (-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>
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
/**
2+
* @name Compiler Removal Of Code To Clear Buffers
3+
* @description --Using the memset function to clear private data as a final expression when working with a variable is potentially dangerous because the compiler can optimize this call.
4+
* --For some compilers, optimization is also possible when using calls to free memory after the memset function.
5+
* --To clear it, you need to use the RtlSecureZeroMemory or memset_s functions, or compilation flags that exclude optimization of memset calls (-fno-builtin-memset).
6+
* @kind problem
7+
* @id cpp/compiler-removal-of-code-to-clear-buffers
8+
* @problem.severity warning
9+
* @precision medium
10+
* @tags security
11+
* external/cwe/cwe-14
12+
*/
13+
14+
import cpp
15+
import semmle.code.cpp.dataflow.DataFlow
16+
17+
/**
18+
* A call to `memset` , for some local variable.
19+
*/
20+
class CompilerRemovaMemset extends FunctionCall {
21+
CompilerRemovaMemset() {
22+
this.getTarget().hasName("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+
source.asExpr() = exp and
27+
exp.getLocation().getEndLine() < this.getArgument(0).getLocation().getStartLine() and
28+
sink.asExpr() = this.getArgument(0)
29+
)
30+
}
31+
32+
predicate isExistsAllocForThisVariable() {
33+
exists(FunctionCall alloc, Variable v |
34+
alloc = v.getAnAssignedValue() and
35+
this.getArgument(0) = v.getAnAccess() and
36+
alloc.getASuccessor+() = this
37+
)
38+
}
39+
40+
predicate isExistsFreeForThisVariable() {
41+
exists(FunctionCall free, Variable v |
42+
free instanceof DeallocationExpr and
43+
this.getArgument(0) = v.getAnAccess() and
44+
free.getArgument(0) = v.getAnAccess() and
45+
this.getASuccessor+() = free
46+
)
47+
}
48+
49+
predicate isExistsCallWithThisVariableExcludingDeallocationCalls() {
50+
exists(FunctionCall fc, Variable v |
51+
not fc instanceof DeallocationExpr and
52+
this.getArgument(0) = v.getAnAccess() and
53+
fc.getAnArgument() = v.getAnAccess() and
54+
this.getASuccessor+() = fc
55+
)
56+
}
57+
58+
predicate isVariableUseAfterMemsetExcludingCalls() {
59+
exists(DataFlow::Node source, DataFlow::Node sink, LocalVariable isv, Expr exp |
60+
DataFlow::localFlow(source, sink) and
61+
this.getArgument(0) = isv.getAnAccess() and
62+
source.asExpr() = isv.getAnAccess() and
63+
exp.getLocation().getStartLine() > this.getArgument(2).getLocation().getEndLine() and
64+
not exp.getParent() instanceof FunctionCall and
65+
sink.asExpr() = exp
66+
)
67+
}
68+
69+
predicate isVariableUseBoundWithArgumentFunction() {
70+
exists(DataFlow::Node source, DataFlow::Node sink, LocalVariable isv, Parameter p, Expr exp |
71+
DataFlow::localFlow(source, sink) and
72+
this.getArgument(0) = isv.getAnAccess() and
73+
this.getEnclosingFunction().getAParameter() = p and
74+
exp.getAChild*() = p.getAnAccess() and
75+
source.asExpr() = exp and
76+
sink.asExpr() = isv.getAnAccess()
77+
)
78+
}
79+
80+
predicate isVariableUseBoundWithGlobalVariable() {
81+
exists(
82+
DataFlow::Node source, DataFlow::Node sink, LocalVariable isv, GlobalVariable gv, Expr exp
83+
|
84+
DataFlow::localFlow(source, sink) and
85+
this.getArgument(0) = isv.getAnAccess() and
86+
exp.getAChild*() = gv.getAnAccess() and
87+
source.asExpr() = exp and
88+
sink.asExpr() = isv.getAnAccess()
89+
)
90+
}
91+
92+
predicate isExistsCompilationFlagsBlockingRemoval() {
93+
exists(Compilation c |
94+
c.getAFileCompiled() = this.getFile() and
95+
c.getAnArgument() = "-fno-builtin-memset"
96+
)
97+
}
98+
99+
predicate isUseVCCompilation() {
100+
exists(Compilation c |
101+
c.getAFileCompiled() = this.getFile() and
102+
(
103+
c.getArgument(2).toString().matches("%gcc%") or
104+
c.getArgument(2).toString().matches("%g++%") or
105+
c.getArgument(2).toString().matches("%clang%") or
106+
c.getArgument(2).toString() = "--force-recompute"
107+
)
108+
)
109+
}
110+
}
111+
112+
from CompilerRemovaMemset fc
113+
where
114+
not (fc.isExistsAllocForThisVariable() and not fc.isExistsFreeForThisVariable()) and
115+
not (fc.isExistsFreeForThisVariable() and not fc.isUseVCCompilation()) and
116+
not fc.isVariableUseAfterMemsetExcludingCalls() and
117+
not fc.isExistsCallWithThisVariableExcludingDeallocationCalls() and
118+
not fc.isVariableUseBoundWithArgumentFunction() and
119+
not fc.isVariableUseBoundWithGlobalVariable() and
120+
not fc.isExistsCompilationFlagsBlockingRemoval()
121+
select fc.getArgument(0), "this variable will not be cleared"

0 commit comments

Comments
 (0)