Merge pull request #1787 from jbj/ast-field-flow-local-fields

C++: Local field flow using global library
This commit is contained in:
Geoffrey White
2019-09-02 11:17:37 +01:00
committed by GitHub
9 changed files with 2503 additions and 3 deletions

View File

@@ -9,6 +9,7 @@
"cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll",
"cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll",
"cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll",
"cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplLocal.qll",
"cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll",
"cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll",
"cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll",

File diff suppressed because it is too large Load Diff

View File

@@ -443,8 +443,14 @@ private module ThisFlow {
* Holds if data flows from `nodeFrom` to `nodeTo` in exactly one local
* (intra-procedural) step.
*/
cached
predicate localFlowStep(Node nodeFrom, Node nodeTo) {
simpleLocalFlowStep(nodeFrom, nodeTo)
or
// Field flow is not strictly a "step" but covers the whole function
// transitively. There's no way to get a step-like relation out of the global
// data flow library, so we just have to accept some big steps here.
FieldFlow::fieldFlow(nodeFrom, nodeTo)
}
/**
@@ -571,6 +577,51 @@ private predicate exprToDefinitionByReferenceStep(Expr exprIn, Expr argOut) {
)
}
private module FieldFlow {
private import DataFlowImplLocal
private import DataFlowPrivate
/**
* A configuration for finding local-only flow through fields. This uses the
* `Configuration` class in the dedicated `DataFlowImplLocal` copy of the
* shared library that's not user-exposed directly.
*
* To keep the flow local to a single function, we put barriers on parameters
* and return statements. Sources and sinks are the values that go into and
* out of fields, respectively.
*/
private class FieldConfiguration extends Configuration {
FieldConfiguration() { this = "FieldConfiguration" }
override predicate isSource(Node source) {
storeStep(source, _, _)
}
override predicate isSink(Node sink) {
readStep(_, _, sink)
}
override predicate isBarrier(Node node) {
node instanceof ParameterNode
}
override predicate isBarrierOut(Node node) {
node.asExpr().getParent() instanceof ReturnStmt
or
node.asExpr().getParent() instanceof ThrowExpr
}
}
predicate fieldFlow(Node node1, Node node2) {
exists(FieldConfiguration cfg |
cfg.hasFlow(node1, node2)
) and
// This configuration should not be able to cross function boundaries, but
// we double-check here just to be sure.
node1.getFunction() = node2.getFunction()
}
}
VariableAccess getAnAccessToAssignedVariable(Expr assign) {
(
assign instanceof Assignment

View File

@@ -212,6 +212,7 @@ module FlowVar_internal {
or
TBlockVar(SubBasicBlock sbb, Variable v) {
not fullySupportedSsaVariable(v) and
not v instanceof Field and // Fields are interprocedural data flow, not local
reachable(sbb) and
(
initializer(sbb.getANode(), v, _)

View File

@@ -137,7 +137,7 @@ void following_pointers(
sink(sourceStruct1_ptr->m1); // flow
sink(sourceStruct1_ptr->getFirst()); // flow [NOT DETECTED with IR]
sink(sourceStruct1_ptr->m2); // no flow
sink(sourceStruct1.m1); // flow (due to lack of no-alias tracking)
sink(sourceStruct1.m1); // no flow
twoIntFields s = { source(), source() };

View File

@@ -22,7 +22,6 @@
| test.cpp:126:8:126:19 | sourceArray1 | test.cpp:120:9:120:20 | sourceArray1 |
| test.cpp:137:27:137:28 | m1 | test.cpp:136:27:136:32 | call to source |
| test.cpp:138:27:138:34 | call to getFirst | test.cpp:136:27:136:32 | call to source |
| test.cpp:140:22:140:23 | m1 | test.cpp:136:27:136:32 | call to source |
| test.cpp:145:10:145:11 | m2 | test.cpp:142:32:142:37 | call to source |
| test.cpp:153:17:153:18 | m2 | test.cpp:151:35:151:40 | call to source |
| test.cpp:188:8:188:8 | y | test.cpp:186:27:186:32 | call to source |

View File

@@ -9,7 +9,6 @@
| test.cpp:109:9:109:14 | test.cpp:110:10:110:12 | IR only |
| test.cpp:136:27:136:32 | test.cpp:137:27:137:28 | AST only |
| test.cpp:136:27:136:32 | test.cpp:138:27:138:34 | AST only |
| test.cpp:136:27:136:32 | test.cpp:140:22:140:23 | AST only |
| test.cpp:147:42:147:47 | test.cpp:149:18:149:19 | IR only |
| test.cpp:395:17:395:22 | test.cpp:397:10:397:18 | AST only |
| test.cpp:407:13:407:18 | test.cpp:413:10:413:14 | AST only |

View File

@@ -0,0 +1,63 @@
int user_input();
void sink(int);
struct S {
int m1, m2;
};
void pointerSetter(S *s) {
s->m1 = user_input();
}
void referenceSetter(S &s) {
s.m1 = user_input();
}
void copySetter(S s) {
s.m1 = user_input();
}
void callSetters() {
S s1 = { 0, 0 };
S s2 = { 0, 0 };
S s3 = { 0, 0 };
pointerSetter(&s1);
referenceSetter(s2);
copySetter(s3);
sink(s1.m1); // flow
sink(s2.m1); // flow
sink(s3.m1); // no flow
}
void assignAfterAlias() {
S s1 = { 0, 0 };
S &ref1 = s1;
ref1.m1 = user_input();
sink(s1.m1); // flow [FALSE NEGATIVE]
S s2 = { 0, 0 };
S &ref2 = s2;
s2.m1 = user_input();
sink(ref2.m1); // flow [FALSE NEGATIVE]
}
void assignAfterCopy() {
S s1 = { 0, 0 };
S copy1 = s1;
copy1.m1 = user_input();
sink(s1.m1); // no flow
S s2 = { 0, 0 };
S copy2 = s2;
s2.m1 = user_input();
sink(copy2.m1); // no flow
}
void assignBeforeCopy() {
S s2 = { 0, 0 };
s2.m1 = user_input();
S copy2 = s2;
sink(copy2.m1); // flow
}

View File

@@ -88,6 +88,20 @@ edges
| C.cpp:27:8:27:11 | `this` parameter in func [s3, ... (1)] | C.cpp:31:10:31:11 | this [s3, ... (1)] |
| C.cpp:29:10:29:11 | this [s1, ... (1)] | C.cpp:29:10:29:11 | s1 |
| C.cpp:31:10:31:11 | this [s3, ... (1)] | C.cpp:31:10:31:11 | s3 |
| aliasing.cpp:9:3:9:3 | s [post update] [m1, ... (1)] | aliasing.cpp:25:17:25:19 | ref arg & ... [m1, ... (1)] |
| aliasing.cpp:9:3:9:22 | ... = ... [void] | aliasing.cpp:9:3:9:3 | s [post update] [m1, ... (1)] |
| aliasing.cpp:9:11:9:20 | call to user_input [void] | aliasing.cpp:9:3:9:22 | ... = ... [void] |
| aliasing.cpp:13:3:13:3 | s [post update] [m1, ... (1)] | aliasing.cpp:26:19:26:20 | ref arg s2 [m1, ... (1)] |
| aliasing.cpp:13:3:13:21 | ... = ... [void] | aliasing.cpp:13:3:13:3 | s [post update] [m1, ... (1)] |
| aliasing.cpp:13:10:13:19 | call to user_input [void] | aliasing.cpp:13:3:13:21 | ... = ... [void] |
| aliasing.cpp:25:17:25:19 | ref arg & ... [m1, ... (1)] | aliasing.cpp:29:8:29:9 | s1 [m1, ... (1)] |
| aliasing.cpp:26:19:26:20 | ref arg s2 [m1, ... (1)] | aliasing.cpp:30:8:30:9 | s2 [m1, ... (1)] |
| aliasing.cpp:29:8:29:9 | s1 [m1, ... (1)] | aliasing.cpp:29:11:29:12 | m1 |
| aliasing.cpp:30:8:30:9 | s2 [m1, ... (1)] | aliasing.cpp:30:11:30:12 | m1 |
| aliasing.cpp:60:3:60:4 | s2 [post update] [m1, ... (1)] | aliasing.cpp:62:8:62:12 | copy2 [m1, ... (1)] |
| aliasing.cpp:60:3:60:22 | ... = ... [void] | aliasing.cpp:60:3:60:4 | s2 [post update] [m1, ... (1)] |
| aliasing.cpp:60:11:60:20 | call to user_input [void] | aliasing.cpp:60:3:60:22 | ... = ... [void] |
| aliasing.cpp:62:8:62:12 | copy2 [m1, ... (1)] | aliasing.cpp:62:14:62:15 | m1 |
| constructors.cpp:26:15:26:15 | f [a_, ... (1)] | constructors.cpp:28:10:28:10 | f [a_, ... (1)] |
| constructors.cpp:26:15:26:15 | f [b_, ... (1)] | constructors.cpp:29:10:29:10 | f [b_, ... (1)] |
| constructors.cpp:28:10:28:10 | f [a_, ... (1)] | constructors.cpp:28:12:28:12 | call to a |
@@ -148,6 +162,9 @@ edges
| 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] |
| aliasing.cpp:29:11:29:12 | m1 | aliasing.cpp:9:11:9:20 | call to user_input [void] | aliasing.cpp:29:11:29:12 | m1 | m1 flows from $@ | aliasing.cpp:9:11:9:20 | call to user_input [void] | call to user_input [void] |
| aliasing.cpp:30:11:30:12 | m1 | aliasing.cpp:13:10:13:19 | call to user_input [void] | aliasing.cpp:30:11:30:12 | m1 | m1 flows from $@ | aliasing.cpp:13:10:13:19 | call to user_input [void] | call to user_input [void] |
| aliasing.cpp:62:14:62:15 | m1 | aliasing.cpp:60:11:60:20 | call to user_input [void] | aliasing.cpp:62:14:62:15 | m1 | m1 flows from $@ | aliasing.cpp:60:11:60:20 | call to user_input [void] | call to user_input [void] |
| constructors.cpp:28:12:28:12 | call to a | constructors.cpp:34:11:34:20 | call to user_input [void] | constructors.cpp:28:12:28:12 | call to a | call to a flows from $@ | constructors.cpp:34:11:34:20 | call to user_input [void] | call to user_input [void] |
| constructors.cpp:28:12:28:12 | call to a | constructors.cpp:36:11:36:20 | call to user_input [void] | constructors.cpp:28:12:28:12 | call to a | call to a flows from $@ | constructors.cpp:36:11:36:20 | call to user_input [void] | call to user_input [void] |
| constructors.cpp:29:12:29:12 | call to b | constructors.cpp:35:14:35:23 | call to user_input [void] | constructors.cpp:29:12:29:12 | call to b | call to b flows from $@ | constructors.cpp:35:14:35:23 | call to user_input [void] | call to user_input [void] |