Skip to content

C++: Field flow through conflated ChiInstructions #3670

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 8 commits into from
Closed

C++: Field flow through conflated ChiInstructions #3670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
654aa77
C++: Allow field flow through conflated ChiInstructions
MathiasVP Jun 10, 2020
0a8145c
C++: Accept tests
MathiasVP Jun 10, 2020
37e7cff
C++: Accept more tests
MathiasVP Jun 10, 2020
97cb2c3
C++: Only allow write side effect -> chi flow when the chi isn't conf…
MathiasVP Jun 11, 2020
3db5f69
C++: Accept tests
MathiasVP Jun 11, 2020
0a25573
Merge branch 'master' into dataflow-through-conflated-chi
MathiasVP Jun 12, 2020
2c18b91
C++: Port changes to the new store nodes after merging in master
MathiasVP Jun 12, 2020
fbc670e
C++: Accept tests
MathiasVP Jun 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,6 @@ private class StoreChainEndInstructionStoreWithChi extends StoreChainEndInstruct
FieldAddressInstruction fi;

StoreChainEndInstructionStoreWithChi() {
not this.isResultConflated() and
this.getPartial() = store and
fi = skipConversion*(store.getDestinationAddress())
}
Expand Down Expand Up @@ -497,7 +496,6 @@ private class StoreChainEndInstructionSideEffect extends StoreChainEndInstructio
FieldAddressInstruction fi;

StoreChainEndInstructionSideEffect() {
not this.isResultConflated() and
this.getPartial() = sideEffect and
fi = skipConversion*(sideEffect.getArgumentDef())
}
Expand Down
48 changes: 48 additions & 0 deletions cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,21 @@ edges
| by_reference.cpp:130:27:130:27 | inner_nested.a [a] | by_reference.cpp:130:27:130:27 | a |
| by_reference.cpp:134:29:134:29 | a | by_reference.cpp:134:29:134:29 | a |
| by_reference.cpp:134:29:134:29 | inner_nested.a [a] | by_reference.cpp:134:29:134:29 | a |
| qualifiers.cpp:22:5:22:38 | Chi [a] | qualifiers.cpp:23:23:23:23 | a |
| qualifiers.cpp:22:5:22:38 | Store | qualifiers.cpp:22:5:22:38 | a [a] |
| qualifiers.cpp:22:5:22:38 | a [a] | qualifiers.cpp:22:5:22:38 | Chi [a] |
| qualifiers.cpp:22:27:22:36 | call to user_input | qualifiers.cpp:22:5:22:38 | Store |
| qualifiers.cpp:23:23:23:23 | a | qualifiers.cpp:23:23:23:23 | a |
| qualifiers.cpp:42:5:42:40 | Chi [a] | qualifiers.cpp:43:23:43:23 | a |
| qualifiers.cpp:42:5:42:40 | Store | qualifiers.cpp:42:5:42:40 | a [a] |
| qualifiers.cpp:42:5:42:40 | a [a] | qualifiers.cpp:42:5:42:40 | Chi [a] |
| qualifiers.cpp:42:29:42:38 | call to user_input | qualifiers.cpp:42:5:42:40 | Store |
| qualifiers.cpp:43:23:43:23 | a | qualifiers.cpp:43:23:43:23 | a |
| qualifiers.cpp:47:5:47:42 | Chi [a] | qualifiers.cpp:48:23:48:23 | a |
| qualifiers.cpp:47:5:47:42 | Store | qualifiers.cpp:47:5:47:42 | a [a] |
| qualifiers.cpp:47:5:47:42 | a [a] | qualifiers.cpp:47:5:47:42 | Chi [a] |
| qualifiers.cpp:47:31:47:40 | call to user_input | qualifiers.cpp:47:5:47:42 | Store |
| qualifiers.cpp:48:23:48:23 | a | qualifiers.cpp:48:23:48:23 | a |
| simple.cpp:65:5:65:22 | i [i] | simple.cpp:66:12:66:12 | Store [i] |
| simple.cpp:65:11:65:20 | call to user_input | simple.cpp:65:5:65:22 | i [i] |
| simple.cpp:66:12:66:12 | Store [i] | simple.cpp:67:13:67:13 | i |
Expand Down Expand Up @@ -128,6 +143,11 @@ edges
| simple.cpp:167:21:167:30 | call to user_input | simple.cpp:167:5:167:32 | Store |
| simple.cpp:168:12:168:23 | Argument 0 indirection [f] | simple.cpp:159:20:159:24 | *inner [f] |
| simple.cpp:168:12:168:23 | inner [f] | simple.cpp:168:12:168:23 | Argument 0 indirection [f] |
| simple.cpp:173:5:173:29 | Chi [a_] | simple.cpp:174:18:174:19 | a_ |
| simple.cpp:173:5:173:29 | Store | simple.cpp:173:5:173:29 | a_ [a_] |
| simple.cpp:173:5:173:29 | a_ [a_] | simple.cpp:173:5:173:29 | Chi [a_] |
| simple.cpp:173:18:173:27 | call to user_input | simple.cpp:173:5:173:29 | Store |
| simple.cpp:174:18:174:19 | a_ | simple.cpp:174:18:174:19 | a_ |
| struct_init.c:14:24:14:25 | *ab [a] | struct_init.c:15:12:15:12 | a |
| struct_init.c:15:12:15:12 | a | struct_init.c:15:12:15:12 | a |
| struct_init.c:20:20:20:29 | Store | struct_init.c:20:20:20:29 | a [a] |
Expand Down Expand Up @@ -232,6 +252,24 @@ nodes
| by_reference.cpp:134:29:134:29 | a | semmle.label | a |
| by_reference.cpp:134:29:134:29 | a | semmle.label | a |
| by_reference.cpp:134:29:134:29 | inner_nested.a [a] | semmle.label | inner_nested.a [a] |
| qualifiers.cpp:22:5:22:38 | Chi [a] | semmle.label | Chi [a] |
| qualifiers.cpp:22:5:22:38 | Store | semmle.label | Store |
| qualifiers.cpp:22:5:22:38 | a [a] | semmle.label | a [a] |
| qualifiers.cpp:22:27:22:36 | call to user_input | semmle.label | call to user_input |
| qualifiers.cpp:23:23:23:23 | a | semmle.label | a |
| qualifiers.cpp:23:23:23:23 | a | semmle.label | a |
| qualifiers.cpp:42:5:42:40 | Chi [a] | semmle.label | Chi [a] |
| qualifiers.cpp:42:5:42:40 | Store | semmle.label | Store |
| qualifiers.cpp:42:5:42:40 | a [a] | semmle.label | a [a] |
| qualifiers.cpp:42:29:42:38 | call to user_input | semmle.label | call to user_input |
| qualifiers.cpp:43:23:43:23 | a | semmle.label | a |
| qualifiers.cpp:43:23:43:23 | a | semmle.label | a |
| qualifiers.cpp:47:5:47:42 | Chi [a] | semmle.label | Chi [a] |
| qualifiers.cpp:47:5:47:42 | Store | semmle.label | Store |
| qualifiers.cpp:47:5:47:42 | a [a] | semmle.label | a [a] |
| qualifiers.cpp:47:31:47:40 | call to user_input | semmle.label | call to user_input |
| qualifiers.cpp:48:23:48:23 | a | semmle.label | a |
| qualifiers.cpp:48:23:48:23 | a | semmle.label | a |
| simple.cpp:65:5:65:22 | i [i] | semmle.label | i [i] |
| simple.cpp:65:11:65:20 | call to user_input | semmle.label | call to user_input |
| simple.cpp:66:12:66:12 | Store [i] | semmle.label | Store [i] |
Expand Down Expand Up @@ -290,6 +328,12 @@ nodes
| simple.cpp:167:21:167:30 | call to user_input | semmle.label | call to user_input |
| simple.cpp:168:12:168:23 | Argument 0 indirection [f] | semmle.label | Argument 0 indirection [f] |
| simple.cpp:168:12:168:23 | inner [f] | semmle.label | inner [f] |
| simple.cpp:173:5:173:29 | Chi [a_] | semmle.label | Chi [a_] |
| simple.cpp:173:5:173:29 | Store | semmle.label | Store |
| simple.cpp:173:5:173:29 | a_ [a_] | semmle.label | a_ [a_] |
| simple.cpp:173:18:173:27 | call to user_input | semmle.label | call to user_input |
| simple.cpp:174:18:174:19 | a_ | semmle.label | a_ |
| simple.cpp:174:18:174:19 | a_ | semmle.label | a_ |
| struct_init.c:14:24:14:25 | *ab [a] | semmle.label | *ab [a] |
| struct_init.c:15:12:15:12 | a | semmle.label | a |
| struct_init.c:15:12:15:12 | a | semmle.label | a |
Expand Down Expand Up @@ -326,6 +370,9 @@ nodes
| by_reference.cpp:114:29:114:29 | a | by_reference.cpp:84:14:84:23 | call to user_input | by_reference.cpp:114:29:114:29 | a | a flows from $@ | by_reference.cpp:84:14:84:23 | call to user_input | call to user_input |
| by_reference.cpp:130:27:130:27 | a | by_reference.cpp:88:13:88:22 | call to user_input | by_reference.cpp:130:27:130:27 | a | a flows from $@ | by_reference.cpp:88:13:88:22 | call to user_input | call to user_input |
| by_reference.cpp:134:29:134:29 | a | by_reference.cpp:88:13:88:22 | call to user_input | by_reference.cpp:134:29:134:29 | a | a flows from $@ | by_reference.cpp:88:13:88:22 | call to user_input | call to user_input |
| qualifiers.cpp:23:23:23:23 | a | qualifiers.cpp:22:27:22:36 | call to user_input | qualifiers.cpp:23:23:23:23 | a | a flows from $@ | qualifiers.cpp:22:27:22:36 | call to user_input | call to user_input |
| qualifiers.cpp:43:23:43:23 | a | qualifiers.cpp:42:29:42:38 | call to user_input | qualifiers.cpp:43:23:43:23 | a | a flows from $@ | qualifiers.cpp:42:29:42:38 | call to user_input | call to user_input |
| qualifiers.cpp:48:23:48:23 | a | qualifiers.cpp:47:31:47:40 | call to user_input | qualifiers.cpp:48:23:48:23 | a | a flows from $@ | qualifiers.cpp:47:31:47:40 | call to user_input | call to user_input |
| simple.cpp:67:13:67:13 | i | simple.cpp:65:11:65:20 | call to user_input | simple.cpp:67:13:67:13 | i | i flows from $@ | simple.cpp:65:11:65:20 | call to user_input | call to user_input |
| simple.cpp:84:14:84:20 | call to getf2f1 | simple.cpp:83:17:83:26 | call to user_input | simple.cpp:84:14:84:20 | call to getf2f1 | call to getf2f1 flows from $@ | simple.cpp:83:17:83:26 | call to user_input | call to user_input |
| simple.cpp:111:18:111:18 | y | simple.cpp:136:31:136:40 | call to user_input | simple.cpp:111:18:111:18 | y | y flows from $@ | simple.cpp:136:31:136:40 | call to user_input | call to user_input |
Expand All @@ -334,6 +381,7 @@ nodes
| simple.cpp:130:15:130:15 | x | simple.cpp:122:22:122:31 | call to user_input | simple.cpp:130:15:130:15 | x | x flows from $@ | simple.cpp:122:22:122:31 | call to user_input | call to user_input |
| simple.cpp:139:23:139:23 | y | simple.cpp:136:31:136:40 | call to user_input | simple.cpp:139:23:139:23 | y | y flows from $@ | simple.cpp:136:31:136:40 | call to user_input | call to user_input |
| simple.cpp:161:17:161:17 | f | simple.cpp:167:21:167:30 | call to user_input | simple.cpp:161:17:161:17 | f | f flows from $@ | simple.cpp:167:21:167:30 | call to user_input | call to user_input |
| simple.cpp:174:18:174:19 | a_ | simple.cpp:173:18:173:27 | call to user_input | simple.cpp:174:18:174:19 | a_ | a_ flows from $@ | simple.cpp:173:18:173:27 | call to user_input | call to user_input |
| struct_init.c:15:12:15:12 | a | struct_init.c:20:20:20:29 | call to user_input | struct_init.c:15:12:15:12 | a | a flows from $@ | struct_init.c:20:20:20:29 | call to user_input | call to user_input |
| struct_init.c:15:12:15:12 | a | struct_init.c:27:7:27:16 | call to user_input | struct_init.c:15:12:15:12 | a | a flows from $@ | struct_init.c:27:7:27:16 | call to user_input | call to user_input |
| struct_init.c:22:11:22:11 | a | struct_init.c:20:20:20:29 | call to user_input | struct_init.c:22:11:22:11 | a | a flows from $@ | struct_init.c:20:20:20:29 | call to user_input | call to user_input |
Expand Down
8 changes: 2 additions & 6 deletions cpp/ql/test/library-tests/dataflow/fields/partial-definition-diff.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,13 @@
| D.cpp:22:14:22:20 | call to getBox1 | AST only |
| D.cpp:22:25:22:31 | call to getElem | AST only |
| D.cpp:30:5:30:5 | b | AST only |
| D.cpp:30:8:30:10 | box | AST only |
| D.cpp:30:13:30:16 | elem | AST only |
| D.cpp:31:14:31:14 | b | AST only |
| D.cpp:37:5:37:5 | b | AST only |
| D.cpp:37:8:37:10 | box | AST only |
| D.cpp:37:21:37:21 | e | AST only |
| D.cpp:38:14:38:14 | b | AST only |
| D.cpp:44:5:44:5 | b | AST only |
| D.cpp:44:8:44:14 | call to getBox1 | AST only |
| D.cpp:44:19:44:22 | elem | AST only |
| D.cpp:45:14:45:14 | b | AST only |
| D.cpp:51:5:51:5 | b | AST only |
Expand All @@ -122,7 +120,6 @@
| D.cpp:57:5:57:12 | boxfield | AST only |
| D.cpp:58:5:58:12 | boxfield | AST only |
| D.cpp:58:5:58:12 | this | AST only |
| D.cpp:58:15:58:17 | box | AST only |
| D.cpp:58:20:58:23 | elem | AST only |
| D.cpp:59:5:59:7 | this | AST only |
| D.cpp:64:10:64:17 | boxfield | AST only |
Expand Down Expand Up @@ -248,7 +245,6 @@
| qualifiers.cpp:12:56:12:56 | a | AST only |
| qualifiers.cpp:13:57:13:57 | a | AST only |
| qualifiers.cpp:22:5:22:9 | outer | AST only |
| qualifiers.cpp:22:11:22:18 | call to getInner | AST only |
| qualifiers.cpp:22:23:22:23 | a | AST only |
| qualifiers.cpp:23:10:23:14 | outer | AST only |
| qualifiers.cpp:23:16:23:20 | inner | AST only |
Expand All @@ -271,14 +267,12 @@
| qualifiers.cpp:38:10:38:14 | outer | AST only |
| qualifiers.cpp:38:16:38:20 | inner | AST only |
| qualifiers.cpp:38:23:38:23 | a | AST only |
| qualifiers.cpp:42:6:42:22 | * ... | AST only |
| qualifiers.cpp:42:7:42:11 | outer | AST only |
| qualifiers.cpp:42:25:42:25 | a | AST only |
| qualifiers.cpp:43:10:43:14 | outer | AST only |
| qualifiers.cpp:43:16:43:20 | inner | AST only |
| qualifiers.cpp:43:23:43:23 | a | AST only |
| qualifiers.cpp:47:6:47:11 | & ... | AST only |
| qualifiers.cpp:47:15:47:22 | call to getInner | AST only |
| qualifiers.cpp:47:27:47:27 | a | AST only |
| qualifiers.cpp:48:10:48:14 | outer | AST only |
| qualifiers.cpp:48:16:48:20 | inner | AST only |
Expand All @@ -305,6 +299,8 @@
| simple.cpp:144:23:144:30 | & ... | AST only |
| simple.cpp:167:17:167:17 | f | AST only |
| simple.cpp:168:12:168:23 | & ... | AST only |
| simple.cpp:173:5:173:29 | * ... | IR only |
| simple.cpp:173:13:173:14 | a_ | AST only |
| struct_init.c:15:8:15:9 | ab | AST only |
| struct_init.c:15:12:15:12 | a | AST only |
| struct_init.c:16:8:16:9 | ab | AST only |
Expand Down
7 changes: 7 additions & 0 deletions cpp/ql/test/library-tests/dataflow/fields/partial-definition-ir.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@
| D.cpp:11:29:11:32 | this |
| D.cpp:16:21:16:23 | this |
| D.cpp:18:29:18:31 | this |
| D.cpp:30:8:30:10 | box |
| D.cpp:44:8:44:14 | call to getBox1 |
| D.cpp:57:5:57:12 | this |
| D.cpp:58:15:58:17 | box |
| aliasing.cpp:9:3:9:3 | s |
| aliasing.cpp:13:3:13:3 | s |
| aliasing.cpp:17:3:17:3 | s |
Expand Down Expand Up @@ -60,6 +63,9 @@
| qualifiers.cpp:9:30:9:33 | this |
| qualifiers.cpp:12:49:12:53 | inner |
| qualifiers.cpp:13:51:13:55 | inner |
| qualifiers.cpp:22:11:22:18 | call to getInner |
| qualifiers.cpp:42:6:42:22 | * ... |
| qualifiers.cpp:47:15:47:22 | call to getInner |
| simple.cpp:20:24:20:25 | this |
| simple.cpp:21:24:21:25 | this |
| simple.cpp:65:5:65:5 | a |
Expand All @@ -76,4 +82,5 @@
| simple.cpp:167:5:167:9 | outer |
| simple.cpp:167:11:167:15 | inner |
| simple.cpp:168:13:168:17 | outer |
| simple.cpp:173:6:173:9 | * ... |
| struct_init.c:36:11:36:15 | outer |
1 change: 1 addition & 0 deletions cpp/ql/test/library-tests/dataflow/fields/partial-definition.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@
| simple.cpp:167:17:167:17 | f |
| simple.cpp:168:12:168:23 | & ... |
| simple.cpp:168:13:168:17 | outer |
| simple.cpp:173:13:173:14 | a_ |
| struct_init.c:15:8:15:9 | ab |
| struct_init.c:15:12:15:12 | a |
| struct_init.c:16:8:16:9 | ab |
Expand Down
6 changes: 3 additions & 3 deletions cpp/ql/test/library-tests/dataflow/fields/qualifiers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace qualifiers {

void assignToGetter(Outer outer) {
outer.getInner()->a = user_input();
sink(outer.inner->a); // $ast $f-:ir
sink(outer.inner->a); // $ast,ir
}

void getterArgument1(Outer outer) {
Expand All @@ -40,11 +40,11 @@ namespace qualifiers {

void assignToGetterStar(Outer outer) {
(*outer.getInner()).a = user_input();
sink(outer.inner->a); // $ast $f-:ir
sink(outer.inner->a); // $ast,ir
}

void assignToGetterAmp(Outer outer) {
(&outer)->getInner()->a = user_input();
sink(outer.inner->a); // $ast $f-:ir
sink(outer.inner->a); // $ast,ir
}
}
10 changes: 8 additions & 2 deletions cpp/ql/test/library-tests/dataflow/fields/simple.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ void sink(int x)

class Foo
{
public:
int a_;
int b_;

public:
int a() { return a_; }
int b() { return b_; }
void setA(int a) { a_ = a; }
Expand Down Expand Up @@ -168,4 +168,10 @@ void test()
read_f(&outer.inner);
}

} // namespace Simple
void false_positive_flow_through_conflated_chi(Foo **pp1, Foo **pp2)
{
(*pp1)->a_ = user_input();
sink((*pp2)->a_); //$f+:ir
}

} // namespace Simple