hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-28 02:05:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

C++: Field flow through ConstructorFieldInit

Browse Source 
This allows a member initializer list to be seen as a sequence of field
assignments. For example, the constructor

    C() : a(taint()) { }

now has data flow similar to

    C() { this.a = taint(); }
This commit is contained in:
Jonas Jensen
2019-08-15 08:56:05 +02:00
parent 1be2380511
commit 45eefdb218
8 changed files with 90 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll
View File

@@ -178,6 +178,12 @@ predicate storeStep(Node node1, Content f, PostUpdateNode node2) {
node2.getPreUpdateNode().asExpr() = fa.getQualifier() and
f.(FieldContent).getField() = fa.getTarget()
)
or
exists(ConstructorFieldInit cfi |
node2.getPreUpdateNode().(PreConstructorInitThis).getConstructorFieldInit() = cfi and
f.(FieldContent).getField() = cfi.getTarget() and
node1.asExpr() = cfi.getExpr()
)
}
/**

56
cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
View File

@@ -13,6 +13,8 @@ private newtype TNode =
TPreConstructorCallNode(ConstructorCall call) or
TExplicitParameterNode(Parameter p) { exists(p.getFunction().getBlock()) } or
TInstanceParameterNode(MemberFunction f) { exists(f.getBlock()) and not f.isStatic() } or
TPreConstructorInitThis(ConstructorFieldInit cfi) or
TPostConstructorInitThis(ConstructorFieldInit cfi) or
TUninitializedNode(LocalVariable v) { not v.hasInitializer() }
/**
@@ -288,6 +290,44 @@ class PreConstructorCallNode extends Node, TPreConstructorCallNode {
override string toString() { result = getConstructorCall().toString() + " [pre constructor call]" }
}
/**
* A synthetic data-flow node that plays the role of the post-update `this`
* pointer in a `ConstructorFieldInit`. For example, the `x(1)` in
* `C() : x(1) { }` is roughly equivalent to `this.x = 1`, and this node is
* equivalent to the `this` _after_ the field has been assigned.
*/
private class PostConstructorInitThis extends PostUpdateNode, TPostConstructorInitThis {
override PreConstructorInitThis getPreUpdateNode() {
this = TPostConstructorInitThis(result.getConstructorFieldInit())
}
override string toString() {
result = getPreUpdateNode().getConstructorFieldInit().toString() + " [post-this]"
}
}
/**
* INTERNAL: do not use.
*
* A synthetic data-flow node that plays the role of the pre-update `this`
* pointer in a `ConstructorFieldInit`. For example, the `x(1)` in
* `C() : x(1) { }` is roughly equivalent to `this.x = 1`, and this node is
* equivalent to the `this` _before_ the field has been assigned.
*/
class PreConstructorInitThis extends Node, TPreConstructorInitThis {
ConstructorFieldInit getConstructorFieldInit() { this = TPreConstructorInitThis(result) }
override Constructor getFunction() { result = getConstructorFieldInit().getEnclosingFunction() }
override PointerType getType() {
result.getBaseType() = getConstructorFieldInit().getEnclosingFunction().getDeclaringType()
}
override Location getLocation() { result = getConstructorFieldInit().getLocation() }
override string toString() { result = getConstructorFieldInit().toString() + " [pre-this]" }
}
/**
* Gets the `Node` corresponding to `e`.
*/
@@ -313,6 +353,12 @@ DefinitionByReferenceNode definitionByReferenceNodeFromArgument(Expr argument) {
UninitializedNode uninitializedNode(LocalVariable v) { result.getLocalVariable() = v }
private module ThisFlow {
/**
* Gets the 0-based index of `thisNode` in `b`, where `thisNode` is an access
* to `this` that may or may not have an associated `PostUpdateNode`. To make
* room for synthetic nodes that access `this`, the index may not correspond
* to an actual `ControlFlowNode`.
*/
private int basicBlockThisIndex(BasicBlock b, Node thisNode) {
// The implicit `this` parameter node is given a very negative offset to
// make space for any `ConstructorFieldInit`s there may be between it and
@@ -320,6 +366,16 @@ private module ThisFlow {
thisNode.(ImplicitParameterNode).getFunction().getBlock() = b and
result = -1000
or
// Place the synthetic `this` node for a `ConstructorFieldInit` at a
// negative offset in the first basic block, between the
// `ImplicitParameterNode` and the first statement.
exists(Constructor constructor, int i |
thisNode.(PreConstructorInitThis).getConstructorFieldInit() =
constructor.getInitializer(i) and
result = -999 + i and
b = thisNode.getFunction().getBlock()
)
or
b.getNode(result) = thisNode.asExpr().(ThisExpr)
}

2
cpp/ql/test/library-tests/dataflow/fields/C.cpp
View File

@@ -26,7 +26,7 @@ public:
void func()
{
sink(s1); // flow [NOT DETECTED] (in either place)
sink(s1); // flow
sink(s2); // flow [NOT DETECTED]
sink(s3); // flow
sink(s4); // flow [NOT DETECTED]

7
cpp/ql/test/library-tests/dataflow/fields/flow.expected
View File

@@ -75,12 +75,18 @@ edges
| B.cpp:18:14:18:17 | box1 [elem1, ... (1)] | B.cpp:18:20:18:24 | elem1 |
| B.cpp:19:10:19:11 | b2 [box1, ... (2)] | B.cpp:19:14:19:17 | box1 [elem2, ... (1)] |
| B.cpp:19:14:19:17 | box1 [elem2, ... (1)] | B.cpp:19:20:19:24 | elem2 |
| C.cpp:18:12:18:18 | call to C [s1, ... (1)] | C.cpp:19:5:19:5 | c [s1, ... (1)] |
| C.cpp:18:12:18:18 | call to C [s3, ... (1)] | C.cpp:19:5:19:5 | c [s3, ... (1)] |
| C.cpp:19:5:19:5 | c [s1, ... (1)] | C.cpp:27:8:27:11 | `this` parameter in func [s1, ... (1)] |
| C.cpp:19:5:19:5 | c [s3, ... (1)] | C.cpp:27:8:27:11 | `this` parameter in func [s3, ... (1)] |
| C.cpp:22:9:22:22 | constructor init of field s1 [post-this] [s1, ... (1)] | C.cpp:18:12:18:18 | call to C [s1, ... (1)] |
| C.cpp:22:12:22:21 | new [void] | C.cpp:22:9:22:22 | constructor init of field s1 [post-this] [s1, ... (1)] |
| C.cpp:24:5:24:8 | this [post update] [s3, ... (1)] | C.cpp:18:12:18:18 | call to C [s3, ... (1)] |
| C.cpp:24:5:24:25 | ... = ... [void] | C.cpp:24:5:24:8 | this [post update] [s3, ... (1)] |
| C.cpp:24:16:24:25 | new [void] | C.cpp:24:5:24:25 | ... = ... [void] |
| C.cpp:27:8:27:11 | `this` parameter in func [s1, ... (1)] | file://:0:0:0:0 | this [s1, ... (1)] |
| C.cpp:27:8:27:11 | `this` parameter in func [s3, ... (1)] | file://:0:0:0:0 | this [s3, ... (1)] |
| file://:0:0:0:0 | this [s1, ... (1)] | C.cpp:29:10:29:11 | s1 |
| file://:0:0:0:0 | this [s3, ... (1)] | C.cpp:31:10:31:11 | s3 |
| simple.cpp:26:15:26:15 | f [a_, ... (1)] | simple.cpp:28:10:28:10 | f [a_, ... (1)] |
| simple.cpp:26:15:26:15 | f [b_, ... (1)] | simple.cpp:29:10:29:10 | f [b_, ... (1)] |
@@ -116,6 +122,7 @@ edges
| B.cpp:10:20:10:24 | elem2 | B.cpp:6:15:6:24 | new [void] | B.cpp:10:20:10:24 | elem2 | elem2 flows from $@ | B.cpp:6:15:6:24 | new [void] | new [void] |
| B.cpp:18:20:18:24 | elem1 | B.cpp:15:15:15:27 | new [void] | B.cpp:18:20:18:24 | elem1 | elem1 flows from $@ | B.cpp:15:15:15:27 | new [void] | new [void] |
| B.cpp:19:20:19:24 | elem2 | B.cpp:15:15:15:27 | new [void] | B.cpp:19:20:19:24 | elem2 | elem2 flows from $@ | B.cpp:15:15:15:27 | new [void] | new [void] |
| C.cpp:29:10:29:11 | s1 | C.cpp:22:12:22:21 | new [void] | C.cpp:29:10:29:11 | s1 | s1 flows from $@ | C.cpp:22:12:22:21 | new [void] | new [void] |
| C.cpp:31:10:31:11 | s3 | C.cpp:24:16:24:25 | new [void] | C.cpp:31:10:31:11 | s3 | s3 flows from $@ | C.cpp:24:16:24:25 | new [void] | new [void] |
| simple.cpp:28:12:28:12 | call to a | simple.cpp:39:12:39:21 | call to user_input [void] | simple.cpp:28:12:28:12 | call to a | call to a flows from $@ | simple.cpp:39:12:39:21 | call to user_input [void] | call to user_input [void] |
| simple.cpp:28:12:28:12 | call to a | simple.cpp:41:12:41:21 | call to user_input [void] | simple.cpp:28:12:28:12 | call to a | call to a flows from $@ | simple.cpp:41:12:41:21 | call to user_input [void] | call to user_input [void] |

15
cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
View File

@@ -62,8 +62,12 @@
| taint.cpp:37:12:37:20 | call to increment | taint.cpp:43:7:43:13 | global9 | |
| taint.cpp:38:13:38:16 | call to zero | taint.cpp:38:2:38:26 | ... = ... | |
| taint.cpp:38:13:38:16 | call to zero | taint.cpp:44:7:44:14 | global10 | |
| taint.cpp:71:2:71:8 | `this` parameter in MyClass | file://:0:0:0:0 | this | |
| taint.cpp:71:2:71:8 | `this` parameter in MyClass | taint.cpp:71:14:71:17 | constructor init of field a [pre-this] | |
| taint.cpp:71:14:71:17 | 0 | taint.cpp:71:14:71:17 | constructor init of field a | TAINT |
| taint.cpp:71:14:71:17 | constructor init of field a [post-this] | taint.cpp:71:20:71:30 | constructor init of field b [pre-this] | |
| taint.cpp:71:14:71:17 | constructor init of field a [pre-this] | taint.cpp:71:20:71:30 | constructor init of field b [pre-this] | |
| taint.cpp:71:20:71:30 | constructor init of field b [post-this] | file://:0:0:0:0 | this | |
| taint.cpp:71:20:71:30 | constructor init of field b [pre-this] | file://:0:0:0:0 | this | |
| taint.cpp:71:22:71:27 | call to source | taint.cpp:71:20:71:30 | constructor init of field b | TAINT |
| taint.cpp:72:3:72:3 | this [post update] | file://:0:0:0:0 | this | |
| taint.cpp:72:7:72:12 | call to source | taint.cpp:72:3:72:14 | ... = ... | |
@@ -212,6 +216,8 @@
| taint.cpp:226:9:226:10 | 0 | taint.cpp:261:7:261:7 | w | |
| taint.cpp:228:11:228:11 | Unknown literal | taint.cpp:228:11:228:11 | constructor init of field t | TAINT |
| taint.cpp:228:11:228:11 | Unknown literal | taint.cpp:228:11:228:11 | constructor init of field u | TAINT |
| taint.cpp:228:11:228:11 | `this` parameter in (constructor) | taint.cpp:228:11:228:11 | constructor init of field t [pre-this] | |
| taint.cpp:228:11:228:11 | `this` parameter in (constructor) | taint.cpp:243:11:243:11 | constructor init of field t [pre-this] | |
| taint.cpp:228:11:232:2 | [...](...){...} | taint.cpp:233:7:233:7 | a | |
| taint.cpp:228:11:232:2 | {...} | taint.cpp:228:11:232:2 | [...](...){...} | TAINT |
| taint.cpp:228:12:228:12 | t | taint.cpp:228:11:232:2 | {...} | TAINT |
@@ -221,12 +227,19 @@
| taint.cpp:235:11:235:11 | Unknown literal | taint.cpp:235:11:235:11 | constructor init of field t | TAINT |
| taint.cpp:235:11:235:11 | Unknown literal | taint.cpp:235:11:235:11 | constructor init of field u | TAINT |
| taint.cpp:235:11:235:11 | Unknown literal | taint.cpp:235:11:235:11 | constructor init of field v | TAINT |
| taint.cpp:235:11:235:11 | `this` parameter in (constructor) | taint.cpp:235:11:235:11 | constructor init of field t [pre-this] | |
| taint.cpp:235:11:235:11 | constructor init of field t [post-this] | taint.cpp:235:11:235:11 | constructor init of field u [pre-this] | |
| taint.cpp:235:11:235:11 | constructor init of field t [pre-this] | taint.cpp:235:11:235:11 | constructor init of field u [pre-this] | |
| taint.cpp:235:11:235:11 | constructor init of field u [post-this] | taint.cpp:235:11:235:11 | constructor init of field v [pre-this] | |
| taint.cpp:235:11:235:11 | constructor init of field u [pre-this] | taint.cpp:235:11:235:11 | constructor init of field v [pre-this] | |
| taint.cpp:235:11:239:2 | [...](...){...} | taint.cpp:240:2:240:2 | b | |
| taint.cpp:235:11:239:2 | {...} | taint.cpp:235:11:239:2 | [...](...){...} | TAINT |
| taint.cpp:235:15:235:15 | `this` parameter in operator() | file://:0:0:0:0 | this | |
| taint.cpp:238:7:238:12 | call to source | taint.cpp:238:3:238:14 | ... = ... | |
| taint.cpp:243:11:243:11 | Unknown literal | taint.cpp:243:11:243:11 | constructor init of field t | TAINT |
| taint.cpp:243:11:243:11 | Unknown literal | taint.cpp:243:11:243:11 | constructor init of field u | TAINT |
| taint.cpp:243:11:243:11 | `this` parameter in (constructor) | taint.cpp:228:11:228:11 | constructor init of field t [pre-this] | |
| taint.cpp:243:11:243:11 | `this` parameter in (constructor) | taint.cpp:243:11:243:11 | constructor init of field t [pre-this] | |
| taint.cpp:243:11:246:2 | [...](...){...} | taint.cpp:247:2:247:2 | c | |
| taint.cpp:243:11:246:2 | {...} | taint.cpp:243:11:246:2 | [...](...){...} | TAINT |
| taint.cpp:243:15:243:15 | `this` parameter in operator() | file://:0:0:0:0 | this | |

4
cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp
View File

@@ -86,11 +86,11 @@ void class_field_test() {
mc1.myMethod();
sink(mc1.a);
sink(mc1.b); // tainted [NOT DETECTED]
sink(mc1.b); // tainted [NOT DETECTED with IR]
sink(mc1.c); // tainted [NOT DETECTED with IR]
sink(mc1.d); // tainted [NOT DETECTED with IR]
sink(mc2.a);
sink(mc2.b); // tainted [NOT DETECTED]
sink(mc2.b); // tainted [NOT DETECTED with IR]
sink(mc2.c); // tainted [NOT DETECTED with IR]
sink(mc2.d);
}

2
cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected
View File

@@ -10,8 +10,10 @@
| taint.cpp:41:7:41:13 | global7 | taint.cpp:35:12:35:17 | call to source |
| taint.cpp:42:7:42:13 | global8 | taint.cpp:35:12:35:17 | call to source |
| taint.cpp:43:7:43:13 | global9 | taint.cpp:37:22:37:27 | call to source |
| taint.cpp:89:11:89:11 | b | taint.cpp:71:22:71:27 | call to source |
| taint.cpp:90:11:90:11 | c | taint.cpp:72:7:72:12 | call to source |
| taint.cpp:91:11:91:11 | d | taint.cpp:77:7:77:12 | call to source |
| taint.cpp:93:11:93:11 | b | taint.cpp:71:22:71:27 | call to source |
| taint.cpp:94:11:94:11 | c | taint.cpp:72:7:72:12 | call to source |
| taint.cpp:129:7:129:9 | * ... | taint.cpp:120:11:120:16 | call to source |
| taint.cpp:134:7:134:9 | * ... | taint.cpp:120:11:120:16 | call to source |

2
cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected
View File

@@ -2,8 +2,10 @@
| taint.cpp:41:7:41:13 | taint.cpp:35:12:35:17 | AST only |
| taint.cpp:42:7:42:13 | taint.cpp:35:12:35:17 | AST only |
| taint.cpp:43:7:43:13 | taint.cpp:37:22:37:27 | AST only |
| taint.cpp:89:11:89:11 | taint.cpp:71:22:71:27 | AST only |
| taint.cpp:90:11:90:11 | taint.cpp:72:7:72:12 | AST only |
| taint.cpp:91:11:91:11 | taint.cpp:77:7:77:12 | AST only |
| taint.cpp:93:11:93:11 | taint.cpp:71:22:71:27 | AST only |
| taint.cpp:94:11:94:11 | taint.cpp:72:7:72:12 | AST only |
| taint.cpp:130:7:130:9 | taint.cpp:127:8:127:13 | IR only |
| taint.cpp:137:7:137:9 | taint.cpp:120:11:120:16 | AST only |