Skip to content

Commit bf526b1

Browse files
author
Paolo Tranquilli
committed
Merge branch 'main' into redsun82/cargo-upgrade-2
2 parents 43bb039 + df28e3b commit bf526b1

File tree

34 files changed

+53515
-82
lines changed

34 files changed

+53515
-82
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: fix
3+
---
4+
* Fixed a problem where `asExpr()` on `DataFlow::Node` would never return `ClassAggregateLiteral`s.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: feature
3+
---
4+
* Added support for `wmain` as part of the ArgvSource model.

cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPCipherOperation.qll

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -67,37 +67,42 @@ abstract class EVP_Cipher_Operation extends OpenSSLOperation, Crypto::KeyOperati
6767
}
6868
}
6969

70-
// abstract class EVP_Update_Call extends EVP_Cipher_Operation { }
71-
abstract class EVP_Final_Call extends EVP_Cipher_Operation {
72-
override Expr getInputArg() { none() }
73-
}
74-
75-
// TODO: only model Final (model final as operation and model update but not as an operation)
76-
// Updates are multiple input consumers (most important)
77-
// TODO: assuming update doesn't ouput, otherwise it outputs artifacts, but is not an operation
7870
class EVP_Cipher_Call extends EVP_Cipher_Operation {
7971
EVP_Cipher_Call() { this.(Call).getTarget().getName() = "EVP_Cipher" }
8072

8173
override Expr getInputArg() { result = this.(Call).getArgument(2) }
8274
}
8375

84-
// ******* TODO: model UPDATE but not as the core operation, rather a step towards final
85-
// see the JCA
86-
// class EVP_Encrypt_Decrypt_or_Cipher_Update_Call extends EVP_Update_Call {
87-
// EVP_Encrypt_Decrypt_or_Cipher_Update_Call() {
88-
// this.(Call).getTarget().getName() in [
89-
// "EVP_EncryptUpdate", "EVP_DecryptUpdate", "EVP_CipherUpdate"
90-
// ]
91-
// }
92-
// override Expr getInputArg() { result = this.(Call).getArgument(3) }
93-
// }
94-
class EVP_Encrypt_Decrypt_or_Cipher_Final_Call extends EVP_Final_Call {
95-
EVP_Encrypt_Decrypt_or_Cipher_Final_Call() {
76+
// NOTE: not modeled as cipher operations, these are intermediate calls
77+
class EVP_Update_Call extends Call {
78+
EVP_Update_Call() {
79+
this.(Call).getTarget().getName() in [
80+
"EVP_EncryptUpdate", "EVP_DecryptUpdate", "EVP_CipherUpdate"
81+
]
82+
}
83+
84+
Expr getInputArg() { result = this.(Call).getArgument(3) }
85+
86+
DataFlow::Node getInputNode() { result.asExpr() = this.getInputArg() }
87+
88+
Expr getContextArg() { result = this.(Call).getArgument(0) }
89+
}
90+
91+
class EVP_Final_Call extends EVP_Cipher_Operation {
92+
EVP_Final_Call() {
9693
this.(Call).getTarget().getName() in [
9794
"EVP_EncryptFinal_ex", "EVP_DecryptFinal_ex", "EVP_CipherFinal_ex", "EVP_EncryptFinal",
9895
"EVP_DecryptFinal", "EVP_CipherFinal"
9996
]
10097
}
98+
99+
EVP_Update_Call getUpdateCalls() {
100+
CTXFlow::ctxArgFlowsToCtxArg(result.getContextArg(), this.getContextArg())
101+
}
102+
103+
override Expr getInputArg() { result = this.getUpdateCalls().getInputArg() }
104+
105+
override Crypto::ConsumerInputDataFlowNode getInputConsumer() { result = this.getInputNode() }
101106
}
102107

103108
class EVP_PKEY_Operation extends EVP_Cipher_Operation {

cpp/ql/lib/ext/generated/openssl.model.yml

Lines changed: 21584 additions & 0 deletions
Large diffs are not rendered by default.

cpp/ql/lib/ext/generated/sqlite.model.yml

Lines changed: 948 additions & 0 deletions
Large diffs are not rendered by default.

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ExprNodes.qll

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,41 @@ private module Cached {
264264
e = getConvertedResultExpression(node.asInstruction(), n)
265265
}
266266

267+
/**
268+
* The IR doesn't have an instruction `i` for which this holds:
269+
* ```
270+
* i.getUnconvertedResultExpression() instanceof ClassAggregateLiteral
271+
* ```
272+
* and thus we don't automatically get a dataflow node for which:
273+
* ```
274+
* node.asExpr() instanceof ClassAggregateLiteral
275+
* ```
276+
* This is because the IR represents a `ClassAggregateLiteral` as a sequence
277+
* of field writes. To work around this we map `asExpr` on the
278+
* `PostUpdateNode` for the last field write to the class aggregate literal.
279+
*/
280+
private class ClassAggregateInitializerPostUpdateNode extends PostFieldUpdateNode {
281+
ClassAggregateLiteral aggr;
282+
283+
ClassAggregateInitializerPostUpdateNode() {
284+
exists(Node node1, FieldContent fc, int position, StoreInstruction store |
285+
store.getSourceValue().getUnconvertedResultExpression() =
286+
aggr.getFieldExpr(fc.getField(), position) and
287+
node1.asInstruction() = store and
288+
// This is the last field write from the aggregate initialization.
289+
not exists(aggr.getFieldExpr(_, position + 1)) and
290+
storeStep(node1, fc, this)
291+
)
292+
}
293+
294+
ClassAggregateLiteral getClassAggregateLiteral() { result = aggr }
295+
}
296+
297+
private predicate exprNodeShouldBePostUpdateNode(Node node, Expr e, int n) {
298+
node.(ClassAggregateInitializerPostUpdateNode).getClassAggregateLiteral() = e and
299+
n = 0
300+
}
301+
267302
/** Holds if `node` should be an `IndirectInstruction` that maps `node.asIndirectExpr()` to `e`. */
268303
private predicate indirectExprNodeShouldBeIndirectInstruction(
269304
IndirectInstruction node, Expr e, int n, int indirectionIndex
@@ -294,7 +329,8 @@ private module Cached {
294329
exprNodeShouldBeInstruction(_, e, n) or
295330
exprNodeShouldBeOperand(_, e, n) or
296331
exprNodeShouldBeIndirectOutNode(_, e, n) or
297-
exprNodeShouldBeIndirectOperand(_, e, n)
332+
exprNodeShouldBeIndirectOperand(_, e, n) or
333+
exprNodeShouldBePostUpdateNode(_, e, n)
298334
}
299335

300336
private class InstructionExprNode extends ExprNodeBase, InstructionNode {
@@ -442,6 +478,12 @@ private module Cached {
442478
final override Expr getConvertedExpr(int n) { exprNodeShouldBeIndirectOperand(this, result, n) }
443479
}
444480

481+
private class PostUpdateExprNode extends ExprNodeBase instanceof PostUpdateNode {
482+
PostUpdateExprNode() { exprNodeShouldBePostUpdateNode(this, _, _) }
483+
484+
final override Expr getConvertedExpr(int n) { exprNodeShouldBePostUpdateNode(this, result, n) }
485+
}
486+
445487
/**
446488
* An expression, viewed as a node in a data flow graph.
447489
*/

cpp/ql/lib/semmle/code/cpp/security/FlowSources.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,12 @@ private class LocalModelSource extends LocalFlowSource {
5555
}
5656

5757
/**
58-
* A local data flow source that the `argv` parameter to `main`.
58+
* A local data flow source that the `argv` parameter to `main` or `wmain`.
5959
*/
6060
private class ArgvSource extends LocalFlowSource {
6161
ArgvSource() {
6262
exists(Function main, Parameter argv |
63-
main.hasGlobalName("main") and
63+
main.hasGlobalName(["main", "wmain"]) and
6464
main.getParameter(1) = argv and
6565
this.asParameter(2) = argv
6666
)
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added flow model for the `SQLite` and `OpenSSL` libraries. This may result in more alerts when running queries on codebases that use these libraries.

cpp/ql/src/utils/modelgenerator/internal/CaptureModels.qll

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ private import semmle.code.cpp.ir.dataflow.internal.TaintTrackingImplSpecific
1313
private import semmle.code.cpp.dataflow.new.TaintTracking as Tt
1414
private import semmle.code.cpp.dataflow.new.DataFlow as Df
1515
private import codeql.mad.modelgenerator.internal.ModelGeneratorImpl
16+
private import semmle.code.cpp.models.interfaces.Taint as Taint
17+
private import semmle.code.cpp.models.interfaces.DataFlow as DataFlow
1618

1719
/**
1820
* Holds if `f` is a "private" function.
@@ -46,6 +48,19 @@ private predicate isUninterestingForModels(Callable api) {
4648
or
4749
api.isFromUninstantiatedTemplate(_)
4850
or
51+
// No need to generate models for functions modeled by hand in QL
52+
api instanceof Taint::TaintFunction
53+
or
54+
api instanceof DataFlow::DataFlowFunction
55+
or
56+
// Don't generate models for main functions
57+
api.hasGlobalName("main")
58+
or
59+
// Don't generate models for system-provided functions. If we want to
60+
// generate models for these we should use a database containing the
61+
// implementations of those system-provided functions in the source root.
62+
not exists(api.getLocation().getFile().getRelativePath())
63+
or
4964
// Exclude functions in test directories (but not the ones in the CodeQL test directory)
5065
exists(Cpp::File f |
5166
f = api.getFile() and

cpp/ql/test/library-tests/dataflow/asExpr/test.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,20 @@ A& get_ref();
2121
void test2() {
2222
take_ref(get_ref()); // $ asExpr="call to get_ref" asIndirectExpr="call to get_ref"
2323
}
24+
25+
struct S {
26+
int a;
27+
int b;
28+
};
29+
30+
void test_aggregate_literal() {
31+
S s1 = {1, 2}; // $ asExpr=1 asExpr=2 asExpr={...}
32+
const S s2 = {3, 4}; // $ asExpr=3 asExpr=4 asExpr={...}
33+
S s3 = (S){5, 6}; // $ asExpr=5 asExpr=6 asExpr={...}
34+
const S s4 = (S){7, 8}; // $ asExpr=7 asExpr=8 asExpr={...}
35+
36+
S s5 = {.a = 1, .b = 2}; // $ asExpr=1 asExpr=2 asExpr={...}
37+
38+
int xs[] = {1, 2, 3}; // $ asExpr=1 asExpr=2 asExpr=3 MISSING: asExpr={...}
39+
const int ys[] = {[0] = 4, [1] = 5, [0] = 6}; // $ asExpr=4 asExpr=5 asExpr=6 MISSING: asExpr={...}
40+
}

0 commit comments

Comments
 (0)