Skip to content

Commit a07de3f

Browse files
RasmusWLRasmusWL
committed
Merge branch 'main' into emptyRedos
2 parents 383b5f2 + c4322fd commit a07de3f

File tree

154 files changed

+2721
-1045
lines changed

Some content is hidden

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

154 files changed

+2721
-1045
lines changed

‎cpp/change-notes/2021-06-15-path-sensitive-stack-reachability.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
lgtm,codescanning
2+
* The `StackVariableReachability` library now ignores some paths that contain an infeasible combination
3+
of conditionals. These improvements primarily affect the queries `cpp/uninitialized-local` and
4+
`cpp/use-after-free`.

‎cpp/change-notes/2021-06-30-wrong-type-format-argument.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lgtm,codescanning
2+
* The 'Wrong type of arguments to formatting function' (cpp/wrong-type-format-argument) query is now more accepting of the string and character formatting differences between Microsoft and non-Microsoft platforms. There are now fewer false positive results.

‎cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,28 +19,32 @@ import cpp
1919
* Holds if the argument corresponding to the `pos` conversion specifier
2020
* of `ffc` is expected to have type `expected`.
2121
*/
22-
pragma[noopt]
2322
private predicate formattingFunctionCallExpectedType(
2423
FormattingFunctionCall ffc, int pos, Type expected
2524
) {
26-
exists(FormattingFunction f, int i, FormatLiteral fl |
27-
ffc instanceof FormattingFunctionCall and
28-
ffc.getTarget() = f and
29-
f.getFormatParameterIndex() = i and
30-
ffc.getArgument(i) = fl and
31-
fl.getConversionType(pos) = expected
32-
)
25+
ffc.getFormat().(FormatLiteral).getConversionType(pos) = expected
26+
}
27+
28+
/**
29+
* Holds if the argument corresponding to the `pos` conversion specifier
30+
* of `ffc` could alternatively have type `expected`, for example on a different
31+
* platform.
32+
*/
33+
private predicate formattingFunctionCallAlternateType(
34+
FormattingFunctionCall ffc, int pos, Type expected
35+
) {
36+
ffc.getFormat().(FormatLiteral).getConversionTypeAlternate(pos) = expected
3337
}
3438

3539
/**
3640
* Holds if the argument corresponding to the `pos` conversion specifier
37-
* of `ffc` is expected to have type `expected` and the corresponding
38-
* argument `arg` has type `actual`.
41+
* of `ffc` is `arg` and has type `actual`.
3942
*/
4043
pragma[noopt]
41-
predicate formatArgType(FormattingFunctionCall ffc, int pos, Type expected, Expr arg, Type actual) {
44+
predicate formattingFunctionCallActualType(
45+
FormattingFunctionCall ffc, int pos, Expr arg, Type actual
46+
) {
4247
exists(Expr argConverted |
43-
formattingFunctionCallExpectedType(ffc, pos, expected) and
4448
ffc.getConversionArgument(pos) = arg and
4549
argConverted = arg.getFullyConverted() and
4650
actual = argConverted.getType()
@@ -72,7 +76,8 @@ class ExpectedType extends Type {
7276
ExpectedType() {
7377
exists(Type t |
7478
(
75-
formatArgType(_, _, t, _, _) or
79+
formattingFunctionCallExpectedType(_, _, t) or
80+
formattingFunctionCallAlternateType(_, _, t) or
7681
formatOtherArgType(_, _, t, _, _)
7782
) and
7883
this = t.getUnspecifiedType()
@@ -91,7 +96,11 @@ class ExpectedType extends Type {
9196
*/
9297
predicate trivialConversion(ExpectedType expected, Type actual) {
9398
exists(Type exp, Type act |
94-
formatArgType(_, _, exp, _, act) and
99+
(
100+
formattingFunctionCallExpectedType(_, _, exp) or
101+
formattingFunctionCallAlternateType(_, _, exp)
102+
) and
103+
formattingFunctionCallActualType(_, _, _, act) and
95104
expected = exp.getUnspecifiedType() and
96105
actual = act.getUnspecifiedType()
97106
) and
@@ -146,9 +155,13 @@ int sizeof_IntType() { exists(IntType it | result = it.getSize()) }
146155
from FormattingFunctionCall ffc, int n, Expr arg, Type expected, Type actual
147156
where
148157
(
149-
formatArgType(ffc, n, expected, arg, actual) and
158+
formattingFunctionCallExpectedType(ffc, n, expected) and
159+
formattingFunctionCallActualType(ffc, n, arg, actual) and
150160
not exists(Type anyExpected |
151-
formatArgType(ffc, n, anyExpected, arg, actual) and
161+
(
162+
formattingFunctionCallExpectedType(ffc, n, anyExpected) or
163+
formattingFunctionCallAlternateType(ffc, n, anyExpected)
164+
) and
152165
trivialConversion(anyExpected.getUnspecifiedType(), actual.getUnspecifiedType())
153166
)
154167
or

‎cpp/ql/src/experimental/Security/CWE/CWE-758/UndefinedOrImplementationDefinedBehavior.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
intA = ++intA + 1; // BAD: undefined behavior when changing variable `intA`
2+
...
3+
intA++;
4+
intA = intA + 1; // GOOD: correct design
5+
...
6+
char * buff;
7+
...
8+
if(funcAdd(buff)+fucDel(buff)>0) return 1; // BAD: undefined behavior when calling functions to change the `buff` variable
9+
...
10+
intA = funcAdd(buff);
11+
intB = funcDel(buff);
12+
if(intA+intB>0) return 1; // GOOD: correct design

‎cpp/ql/src/experimental/Security/CWE/CWE-758/UndefinedOrImplementationDefinedBehavior.qhelp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>In some situations, the code constructs used may be executed in the wrong order in which the developer designed them. For example, if you call multiple functions as part of a single expression, and the functions have the ability to modify a shared resource, then the sequence in which the resource is changed can be unpredictable. These code snippets look suspicious and require the developer's attention.</p>
7+
8+
9+
</overview>
10+
<recommendation>
11+
12+
<p>We recommend that you use more guaranteed, in terms of sequence of execution, coding techniques.</p>
13+
14+
</recommendation>
15+
<example>
16+
<p>The following example demonstrates sections of code with insufficient execution sequence definition.</p>
17+
<sample src="UndefinedOrImplementationDefinedBehavior.c" />
18+
19+
</example>
20+
<references>
21+
22+
<li>
23+
CWE Common Weakness Enumeration:
24+
<a href="https://wiki.sei.cmu.edu/confluence/display/c/EXP10-C.+Do+not+depend+on+the+order+of+evaluation+of+subexpressions+or+the+order+in+which+side+effects+take+place"> EXP10-C. Do not depend on the order of evaluation of subexpressions or the order in which side effects take place</a>.
25+
</li>
26+
27+
</references>
28+
</qhelp>

‎cpp/ql/src/experimental/Security/CWE/CWE-758/UndefinedOrImplementationDefinedBehavior.ql

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
/**
2+
* @name Errors Of Undefined Program Behavior
3+
* @description --In some situations, the code constructs used may be executed in the wrong order in which the developer designed them.
4+
* --For example, if you call multiple functions as part of a single expression, and the functions have the ability to modify a shared resource, then the sequence in which the resource is changed can be unpredictable.
5+
* --These code snippets look suspicious and require the developer's attention.
6+
* @kind problem
7+
* @id cpp/errors-of-undefined-program-behavior
8+
* @problem.severity warning
9+
* @precision medium
10+
* @tags security
11+
* external/cwe/cwe-758
12+
*/
13+
14+
import cpp
15+
import semmle.code.cpp.valuenumbering.HashCons
16+
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
17+
18+
/**
19+
* Threatening expressions of undefined behavior.
20+
*/
21+
class ExpressionsOfTheSameLevel extends Expr {
22+
Expr exp2;
23+
24+
ExpressionsOfTheSameLevel() {
25+
this != exp2 and
26+
this.getParent() = exp2.getParent()
27+
}
28+
29+
/** Holds if the underlying expression is a function call. */
30+
predicate expressionCall() {
31+
this instanceof FunctionCall and
32+
exp2.getAChild*() instanceof FunctionCall and
33+
not this.getParent() instanceof Operator and
34+
not this.(FunctionCall).hasQualifier()
35+
}
36+
37+
/** Holds if the underlying expression is a call to a function to free resources. */
38+
predicate existsCloseOrFreeCall() {
39+
(
40+
globalValueNumber(this.(FunctionCall).getAnArgument()) =
41+
globalValueNumber(exp2.getAChild*().(FunctionCall).getAnArgument()) or
42+
hashCons(this.(FunctionCall).getAnArgument()) =
43+
hashCons(exp2.getAChild*().(FunctionCall).getAnArgument())
44+
) and
45+
(
46+
this.(FunctionCall).getTarget().hasGlobalOrStdName("close") or
47+
this.(FunctionCall).getTarget().hasGlobalOrStdName("free") or
48+
this.(FunctionCall).getTarget().hasGlobalOrStdName("fclose")
49+
)
50+
}
51+
52+
/** Holds if the arguments in the function can be changed. */
53+
predicate generalArgumentDerivedType() {
54+
exists(Parameter prt1, Parameter prt2, AssignExpr aet1, AssignExpr aet2, int i, int j |
55+
not this.(FunctionCall).getArgument(i).isConstant() and
56+
hashCons(this.(FunctionCall).getArgument(i)) =
57+
hashCons(exp2.getAChild*().(FunctionCall).getArgument(j)) and
58+
prt1 = this.(FunctionCall).getTarget().getParameter(i) and
59+
prt2 = exp2.getAChild*().(FunctionCall).getTarget().getParameter(j) and
60+
prt1.getType() instanceof DerivedType and
61+
(
62+
aet1 = this.(FunctionCall).getTarget().getEntryPoint().getASuccessor*() and
63+
(
64+
aet1.getLValue().(ArrayExpr).getArrayBase().(VariableAccess).getTarget() =
65+
prt1.getAnAccess().getTarget() or
66+
aet1.getLValue().(VariableAccess).getTarget() = prt1.getAnAccess().getTarget()
67+
)
68+
or
69+
exists(FunctionCall fc1 |
70+
fc1.getTarget().hasGlobalName("memcpy") and
71+
fc1.getArgument(0).(VariableAccess).getTarget() = prt1.getAnAccess().getTarget() and
72+
fc1 = this.(FunctionCall).getTarget().getEntryPoint().getASuccessor*()
73+
)
74+
) and
75+
(
76+
aet2 = exp2.getAChild*().(FunctionCall).getTarget().getEntryPoint().getASuccessor*() and
77+
(
78+
aet2.getLValue().(ArrayExpr).getArrayBase().(VariableAccess).getTarget() =
79+
prt2.getAnAccess().getTarget() or
80+
aet2.getLValue().(VariableAccess).getTarget() = prt2.getAnAccess().getTarget()
81+
)
82+
or
83+
exists(FunctionCall fc1 |
84+
fc1.getTarget().hasGlobalName("memcpy") and
85+
fc1.getArgument(0).(VariableAccess).getTarget() = prt2.getAnAccess().getTarget() and
86+
fc1 = exp2.(FunctionCall).getTarget().getEntryPoint().getASuccessor*()
87+
)
88+
)
89+
)
90+
}
91+
92+
/** Holds if functions have a common global argument. */
93+
predicate generalGlobalArgument() {
94+
exists(Declaration dl, AssignExpr aet1, AssignExpr aet2 |
95+
dl instanceof GlobalVariable and
96+
(
97+
(
98+
aet1.getLValue().(Access).getTarget() = dl or
99+
aet1.getLValue().(ArrayExpr).getArrayBase().(VariableAccess).getTarget() = dl
100+
) and
101+
aet1 = this.(FunctionCall).getTarget().getEntryPoint().getASuccessor*() and
102+
not aet1.getRValue().isConstant()
103+
or
104+
exists(FunctionCall fc1 |
105+
fc1.getTarget().hasGlobalName("memcpy") and
106+
fc1.getArgument(0).(VariableAccess).getTarget() = dl and
107+
fc1 = this.(FunctionCall).getTarget().getEntryPoint().getASuccessor*()
108+
)
109+
) and
110+
(
111+
(
112+
aet2.getLValue().(Access).getTarget() = dl or
113+
aet2.getLValue().(ArrayExpr).getArrayBase().(VariableAccess).getTarget() = dl
114+
) and
115+
aet2 = exp2.(FunctionCall).getTarget().getEntryPoint().getASuccessor*()
116+
or
117+
exists(FunctionCall fc1 |
118+
fc1.getTarget().hasGlobalName("memcpy") and
119+
fc1.getArgument(0).(VariableAccess).getTarget() = dl and
120+
fc1 = exp2.(FunctionCall).getTarget().getEntryPoint().getASuccessor*()
121+
)
122+
)
123+
)
124+
}
125+
126+
/** Holds if sequence point is not present in expression. */
127+
predicate orderOfActionExpressions() {
128+
not this.getParent() instanceof BinaryLogicalOperation and
129+
not this.getParent() instanceof ConditionalExpr and
130+
not this.getParent() instanceof Loop and
131+
not this.getParent() instanceof CommaExpr
132+
}
133+
134+
/** Holds if expression is crement. */
135+
predicate dangerousCrementChanges() {
136+
hashCons(this.(CrementOperation).getOperand()) = hashCons(exp2.(CrementOperation).getOperand())
137+
or
138+
hashCons(this.(CrementOperation).getOperand()) = hashCons(exp2)
139+
or
140+
hashCons(this.(CrementOperation).getOperand()) = hashCons(exp2.(ArrayExpr).getArrayOffset())
141+
or
142+
hashCons(this.(Assignment).getLValue()) = hashCons(exp2.(Assignment).getLValue())
143+
or
144+
not this.getAChild*() instanceof Call and
145+
(
146+
hashCons(this.getAChild*().(CrementOperation).getOperand()) = hashCons(exp2) or
147+
hashCons(this.getAChild*().(CrementOperation).getOperand()) =
148+
hashCons(exp2.(Assignment).getLValue())
149+
)
150+
}
151+
}
152+
153+
from ExpressionsOfTheSameLevel eots
154+
where
155+
eots.orderOfActionExpressions() and
156+
(
157+
eots.expressionCall() and
158+
(
159+
eots.generalArgumentDerivedType() or
160+
eots.generalGlobalArgument() or
161+
eots.existsCloseOrFreeCall()
162+
)
163+
or
164+
eots.dangerousCrementChanges()
165+
)
166+
select eots, "This expression may have undefined behavior."

‎cpp/ql/src/semmle/code/cpp/File.qll

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -272,20 +272,16 @@ class File extends Container, @file {
272272
* are compiled by a Microsoft compiler are detected by this predicate.
273273
*/
274274
predicate compiledAsMicrosoft() {
275-
exists(Compilation c |
276-
c.getAFileCompiled() = this and
275+
exists(File f, Compilation c |
276+
c.getAFileCompiled() = f and
277277
(
278278
c.getAnArgument() = "--microsoft" or
279279
c.getAnArgument()
280280
.toLowerCase()
281281
.replaceAll("\\", "/")
282282
.matches(["%/cl.exe", "%/clang-cl.exe"])
283-
)
284-
)
285-
or
286-
exists(File parent |
287-
parent.compiledAsMicrosoft() and
288-
parent.getAnIncludedFile() = this
283+
) and
284+
f.getAnIncludedFile*() = this
289285
)
290286
}
291287

@@ -358,6 +354,11 @@ class File extends Container, @file {
358354
string getShortName() { files(underlyingElement(this), _, result, _, _) }
359355
}
360356

357+
/**
358+
* Holds if any file was compiled by a Microsoft compiler.
359+
*/
360+
predicate anyFileCompiledAsMicrosoft() { any(File f).compiledAsMicrosoft() }
361+
361362
/**
362363
* A C/C++ header file, as determined (mainly) by file extension.
363364
*

‎cpp/ql/src/semmle/code/cpp/commons/Printf.qll

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ class FormatLiteral extends Literal {
306306
* Holds if this `FormatLiteral` is in a context that supports
307307
* Microsoft rules and extensions.
308308
*/
309-
predicate isMicrosoft() { any(File f).compiledAsMicrosoft() }
309+
predicate isMicrosoft() { anyFileCompiledAsMicrosoft() }
310310

311311
/**
312312
* Gets the format string, with '%%' and '%@' replaced by '_' (to avoid processing
@@ -869,6 +869,33 @@ class FormatLiteral extends Literal {
869869
)
870870
}
871871

872+
/**
873+
* Gets an alternate argument type that would be required by the nth
874+
* conversion specifier on a Microsoft or non-Microsoft platform, opposite
875+
* to that of the snapshot. This may be useful for answering 'what might
876+
* happen' questions.
877+
*/
878+
Type getConversionTypeAlternate(int n) {
879+
exists(string len, string conv |
880+
this.parseConvSpec(n, _, _, _, _, _, len, conv) and
881+
(len != "l" and len != "w" and len != "h") and
882+
getUse().getTarget().(FormattingFunction).getFormatCharType().getSize() > 1 and // wide function
883+
(
884+
conv = "c" and
885+
result = getNonDefaultCharType()
886+
or
887+
conv = "C" and
888+
result = getDefaultCharType()
889+
or
890+
conv = "s" and
891+
result.(PointerType).getBaseType() = getNonDefaultCharType()
892+
or
893+
conv = "S" and
894+
result.(PointerType).getBaseType() = getDefaultCharType()
895+
)
896+
)
897+
}
898+
872899
/**
873900
* Holds if the nth conversion specifier of this format string (if `mode = 2`), it's
874901
* minimum field width (if `mode = 0`) or it's precision (if `mode = 1`) requires a

‎cpp/ql/src/semmle/code/cpp/controlflow/DefinitionsAndUses.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ private predicate bbSuccessorEntryReachesDefOrUse(
208208
boolean skipsFirstLoopAlwaysTrueUponEntry
209209
) {
210210
exists(BasicBlock succ, boolean succSkipsFirstLoopAlwaysTrueUponEntry |
211-
bbSuccessorEntryReachesLoopInvariant(bb, succ, skipsFirstLoopAlwaysTrueUponEntry,
211+
bbSuccessorEntryReachesLoopInvariant0(bb, succ, skipsFirstLoopAlwaysTrueUponEntry,
212212
succSkipsFirstLoopAlwaysTrueUponEntry)
213213
|
214214
bbEntryReachesDefOrUseLocally(succ, v, defOrUse) and

0 commit comments

Comments
 (0)