Skip to content

Commit 04db335

Browse files
RasmusWLRasmusWL
committed
Merge branch 'main' into sensitive-improvements
2 parents ea0c1d7 + 716627c commit 04db335

File tree

280 files changed

+9671
-4881
lines changed

Some content is hidden

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

280 files changed

+9671
-4881
lines changed

‎.codeqlmanifest.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
{ "provide": [ "*/ql/src/qlpack.yml",
22
"*/ql/test/qlpack.yml",
3+
"cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/qlpack.yml",
34
"*/ql/examples/qlpack.yml",
45
"*/upgrades/qlpack.yml",
56
"misc/legacy-support/*/qlpack.yml",

‎cpp/ql/src/Security/CWE/CWE-190/TaintedAllocationSize.ql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
* @tags reliability
1010
* security
1111
* external/cwe/cwe-190
12+
* external/cwe/cwe-789
1213
*/
1314

1415
import cpp

‎cpp/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql

Lines changed: 70 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ Function getAnInsecureEncryptionFunction() {
2828
/**
2929
* A function with additional evidence it is related to encryption.
3030
*/
31-
Function getAdditionalEvidenceFunction() {
31+
Function getAnAdditionalEvidenceFunction() {
3232
(
3333
isEncryptionAdditionalEvidence(result.getName()) or
3434
isEncryptionAdditionalEvidence(result.getAParameter().getName())
@@ -47,7 +47,7 @@ Macro getAnInsecureEncryptionMacro() {
4747
/**
4848
* A macro with additional evidence it is related to encryption.
4949
*/
50-
Macro getAdditionalEvidenceMacro() {
50+
Macro getAnAdditionalEvidenceMacro() {
5151
isEncryptionAdditionalEvidence(result.getName()) and
5252
exists(result.getAnInvocation())
5353
}
@@ -63,61 +63,78 @@ EnumConstant getAnInsecureEncryptionEnumConst() { isInsecureEncryption(result.ge
6363
EnumConstant getAdditionalEvidenceEnumConst() { isEncryptionAdditionalEvidence(result.getName()) }
6464

6565
/**
66-
* A function call we have a high confidence is related to use of an insecure
67-
* encryption algorithm.
66+
* A function call we have a high confidence is related to use of an insecure encryption algorithm, along
67+
* with an associated `Element` which might be the best point to blame, and a description of that element.
6868
*/
69-
class InsecureFunctionCall extends FunctionCall {
70-
Element blame;
71-
string explain;
69+
predicate getInsecureEncryptionEvidence(FunctionCall fc, Element blame, string description) {
70+
// find use of an insecure algorithm name
71+
(
72+
fc.getTarget() = getAnInsecureEncryptionFunction() and
73+
blame = fc and
74+
description = "call to " + fc.getTarget().getName()
75+
or
76+
exists(MacroInvocation mi |
77+
(
78+
mi.getAnExpandedElement() = fc or
79+
mi.getAnExpandedElement() = fc.getAnArgument()
80+
) and
81+
mi.getMacro() = getAnInsecureEncryptionMacro() and
82+
blame = mi and
83+
description = "invocation of macro " + mi.getMacro().getName()
84+
)
85+
or
86+
exists(EnumConstantAccess ec |
87+
ec = fc.getAnArgument() and
88+
ec.getTarget() = getAnInsecureEncryptionEnumConst() and
89+
blame = ec and
90+
description = "access of enum constant " + ec.getTarget().getName()
91+
)
92+
) and
93+
// find additional evidence that this function is related to encryption.
94+
(
95+
fc.getTarget() = getAnAdditionalEvidenceFunction()
96+
or
97+
exists(MacroInvocation mi |
98+
(
99+
mi.getAnExpandedElement() = fc or
100+
mi.getAnExpandedElement() = fc.getAnArgument()
101+
) and
102+
mi.getMacro() = getAnAdditionalEvidenceMacro()
103+
)
104+
or
105+
exists(EnumConstantAccess ec |
106+
ec = fc.getAnArgument() and
107+
ec.getTarget() = getAdditionalEvidenceEnumConst()
108+
)
109+
)
110+
}
111+
112+
/**
113+
* An element that is the `blame` of an `InsecureFunctionCall`.
114+
*/
115+
class BlamedElement extends Element {
116+
string description;
117+
118+
BlamedElement() { getInsecureEncryptionEvidence(_, this, description) }
72119

73-
InsecureFunctionCall() {
74-
// find use of an insecure algorithm name
75-
(
76-
getTarget() = getAnInsecureEncryptionFunction() and
77-
blame = this and
78-
explain = "function call"
79-
or
80-
exists(MacroInvocation mi |
81-
(
82-
mi.getAnExpandedElement() = this or
83-
mi.getAnExpandedElement() = this.getAnArgument()
84-
) and
85-
mi.getMacro() = getAnInsecureEncryptionMacro() and
86-
blame = mi and
87-
explain = "macro invocation"
88-
)
89-
or
90-
exists(EnumConstantAccess ec |
91-
ec = this.getAnArgument() and
92-
ec.getTarget() = getAnInsecureEncryptionEnumConst() and
93-
blame = ec and
94-
explain = "enum constant access"
95-
)
96-
) and
97-
// find additional evidence that this function is related to encryption.
98-
(
99-
getTarget() = getAdditionalEvidenceFunction()
100-
or
101-
exists(MacroInvocation mi |
102-
(
103-
mi.getAnExpandedElement() = this or
104-
mi.getAnExpandedElement() = this.getAnArgument()
105-
) and
106-
mi.getMacro() = getAdditionalEvidenceMacro()
107-
)
108-
or
109-
exists(EnumConstantAccess ec |
110-
ec = this.getAnArgument() and
111-
ec.getTarget() = getAdditionalEvidenceEnumConst()
112-
)
120+
/**
121+
* Holds if this is the `num`-th `BlamedElement` in `f`.
122+
*/
123+
predicate hasFileRank(File f, int num) {
124+
exists(int loc |
125+
getLocation().charLoc(f, loc, _) and
126+
loc =
127+
rank[num](BlamedElement other, int loc2 | other.getLocation().charLoc(f, loc2, _) | loc2)
113128
)
114129
}
115130

116-
Element getBlame() { result = blame }
117-
118-
string getDescription() { result = explain }
131+
string getDescription() { result = description }
119132
}
120133

121-
from InsecureFunctionCall c
122-
select c.getBlame(),
123-
"This " + c.getDescription() + " specifies a broken or weak cryptographic algorithm."
134+
from File f, BlamedElement firstResult, BlamedElement thisResult
135+
where
136+
firstResult.hasFileRank(f, 1) and
137+
thisResult.hasFileRank(f, _)
138+
select firstResult,
139+
"This file makes use of a broken or weak cryptographic algorithm (specified by $@).", thisResult,
140+
thisResult.getDescription()

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,9 @@ deprecated library class LocationExpr extends Location, @location_expr { }
128128
* Gets the length of the longest line in file `f`.
129129
*/
130130
pragma[nomagic]
131-
private int maxCols(File f) { result = max(Location l | l.getFile() = f | l.getEndColumn()) }
131+
private int maxCols(File f) {
132+
result = max(Location l | l.getFile() = f | l.getStartColumn().maximum(l.getEndColumn()))
133+
}
132134

133135
/**
134136
* A C/C++ element that has a location in a file

‎cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ predicate addressOperandAllocationAndOffset(
411411
allocation.getABaseInstruction() = base and
412412
hasBaseAndOffset(addrOperand, base, bitOffset) and
413413
not exists(Instruction previousBase |
414-
hasBaseAndOffset(addrOperand, previousBase, _) and
414+
hasBaseAndOffset(addrOperand, pragma[only_bind_out](previousBase), _) and
415415
previousBase = base.getAnOperand().getDef()
416416
)
417417
)

‎cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/SideEffects.qll

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,64 @@ private import semmle.code.cpp.ir.implementation.Opcode
1010
private import semmle.code.cpp.models.interfaces.PointerWrapper
1111
private import semmle.code.cpp.models.interfaces.SideEffect
1212

13+
private predicate isDeeplyConst(Type t) {
14+
t.isConst() and
15+
isDeeplyConstBelow(t)
16+
or
17+
isDeeplyConst(t.(Decltype).getBaseType())
18+
or
19+
isDeeplyConst(t.(ReferenceType).getBaseType())
20+
or
21+
exists(SpecifiedType specType | specType = t |
22+
specType.getASpecifier().getName() = "const" and
23+
isDeeplyConstBelow(specType.getBaseType())
24+
)
25+
or
26+
isDeeplyConst(t.(ArrayType).getBaseType())
27+
}
28+
29+
private predicate isDeeplyConstBelow(Type t) {
30+
t instanceof BuiltInType
31+
or
32+
not t instanceof PointerWrapper and
33+
t instanceof Class
34+
or
35+
t instanceof Enum
36+
or
37+
isDeeplyConstBelow(t.(Decltype).getBaseType())
38+
or
39+
isDeeplyConst(t.(PointerType).getBaseType())
40+
or
41+
isDeeplyConst(t.(ReferenceType).getBaseType())
42+
or
43+
isDeeplyConstBelow(t.(SpecifiedType).getBaseType())
44+
or
45+
isDeeplyConst(t.(ArrayType).getBaseType())
46+
or
47+
isDeeplyConst(t.(GNUVectorType).getBaseType())
48+
or
49+
isDeeplyConst(t.(FunctionPointerIshType).getBaseType())
50+
or
51+
isDeeplyConst(t.(PointerWrapper).getTemplateArgument(0))
52+
or
53+
isDeeplyConst(t.(PointerToMemberType).getBaseType())
54+
or
55+
isDeeplyConstBelow(t.(TypedefType).getBaseType())
56+
}
57+
58+
private predicate isConstPointerLike(Type t) {
59+
(
60+
t instanceof PointerWrapper
61+
or
62+
t instanceof PointerType
63+
or
64+
t instanceof ArrayType
65+
or
66+
t instanceof ReferenceType
67+
) and
68+
isDeeplyConstBelow(t)
69+
}
70+
1371
/**
1472
* Holds if the specified call has a side effect that does not come from a `SideEffectFunction`
1573
* model.
@@ -45,7 +103,7 @@ private predicate hasDefaultSideEffect(Call call, ParameterIndex i, boolean buff
45103
) and
46104
(
47105
isWrite = true and
48-
not call.getTarget().getParameter(i).getType().isDeeplyConstBelow()
106+
not isConstPointerLike(call.getTarget().getParameter(i).getUnderlyingType())
49107
or
50108
isWrite = false
51109
)

‎cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ predicate addressOperandAllocationAndOffset(
411411
allocation.getABaseInstruction() = base and
412412
hasBaseAndOffset(addrOperand, base, bitOffset) and
413413
not exists(Instruction previousBase |
414-
hasBaseAndOffset(addrOperand, previousBase, _) and
414+
hasBaseAndOffset(addrOperand, pragma[only_bind_out](previousBase), _) and
415415
previousBase = base.getAnOperand().getDef()
416416
)
417417
)

0 commit comments

Comments
 (0)