Skip to content

Commit 038bea2

Browse files
committed
C++: Add type check to prevent field conflation
1 parent 250e12a commit 038bea2

File tree

3 files changed

+9
-8
lines changed

3 files changed

+9
-8
lines changed

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -492,8 +492,15 @@ private predicate simpleInstructionLocalFlowStep(Instruction iFrom, Instruction
492492
// `LoadInstruction` together directly, this rule will break if there's any
493493
// reassignment of the parameter indirection, including a conditional one that
494494
// leads to a phi node.
495-
iTo.(LoadInstruction).getSourceValueOperand().getAnyDef() =
496-
iFrom.(InitializeIndirectionInstruction)
495+
exists(InitializeIndirectionInstruction init |
496+
iFrom = init and
497+
iTo.(LoadInstruction).getSourceValueOperand().getAnyDef() = init and
498+
// Check that the types match. Otherwise we can get flow from an object to
499+
// its fields, which leads to field conflation when there's flow from other
500+
// fields to the object elsewhere.
501+
init.getParameter().getType().getUnspecifiedType().(DerivedType).getBaseType() =
502+
iTo.getResultType().getUnspecifiedType()
503+
)
497504
or
498505
// Treat all conversions as flow, even conversions between different numeric types.
499506
iTo.(ConvertInstruction).getUnary() = iFrom

cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.expected

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,15 +115,12 @@
115115
| defaulttainttracking.cpp:113:9:113:14 | call to getenv | defaulttainttracking.cpp:113:9:113:24 | access to array |
116116
| defaulttainttracking.cpp:113:9:113:14 | call to getenv | defaulttainttracking.cpp:114:10:114:10 | x |
117117
| defaulttainttracking.cpp:113:9:113:14 | call to getenv | test_diff.cpp:2:11:2:13 | p#0 |
118-
| defaulttainttracking.cpp:120:11:120:16 | call to getenv | defaulttainttracking.cpp:10:11:10:13 | p#0 |
119118
| defaulttainttracking.cpp:120:11:120:16 | call to getenv | defaulttainttracking.cpp:120:11:120:16 | call to getenv |
120119
| defaulttainttracking.cpp:120:11:120:16 | call to getenv | defaulttainttracking.cpp:120:11:120:26 | (int)... |
121120
| defaulttainttracking.cpp:120:11:120:16 | call to getenv | defaulttainttracking.cpp:120:11:120:26 | access to array |
122121
| defaulttainttracking.cpp:120:11:120:16 | call to getenv | defaulttainttracking.cpp:123:23:123:24 | pp |
123122
| defaulttainttracking.cpp:120:11:120:16 | call to getenv | defaulttainttracking.cpp:124:8:124:9 | pp |
124-
| defaulttainttracking.cpp:120:11:120:16 | call to getenv | defaulttainttracking.cpp:124:12:124:12 | y |
125123
| defaulttainttracking.cpp:120:11:120:16 | call to getenv | defaulttainttracking.cpp:130:13:130:14 | & ... |
126-
| defaulttainttracking.cpp:120:11:120:16 | call to getenv | test_diff.cpp:2:11:2:13 | p#0 |
127124
| globals.cpp:5:20:5:25 | call to getenv | globals.cpp:2:17:2:25 | sinkParam |
128125
| globals.cpp:5:20:5:25 | call to getenv | globals.cpp:5:12:5:16 | local |
129126
| globals.cpp:5:20:5:25 | call to getenv | globals.cpp:5:20:5:25 | call to getenv |

cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/test_diff.expected

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,10 @@
2727
| defaulttainttracking.cpp:113:9:113:14 | call to getenv | defaulttainttracking.cpp:113:5:113:5 | x | AST only |
2828
| defaulttainttracking.cpp:113:9:113:14 | call to getenv | defaulttainttracking.cpp:114:10:114:10 | x | IR only |
2929
| defaulttainttracking.cpp:113:9:113:14 | call to getenv | test_diff.cpp:2:11:2:13 | p#0 | IR only |
30-
| defaulttainttracking.cpp:120:11:120:16 | call to getenv | defaulttainttracking.cpp:10:11:10:13 | p#0 | IR only |
3130
| defaulttainttracking.cpp:120:11:120:16 | call to getenv | defaulttainttracking.cpp:120:7:120:7 | x | AST only |
3231
| defaulttainttracking.cpp:120:11:120:16 | call to getenv | defaulttainttracking.cpp:123:23:123:24 | pp | IR only |
3332
| defaulttainttracking.cpp:120:11:120:16 | call to getenv | defaulttainttracking.cpp:124:8:124:9 | pp | IR only |
34-
| defaulttainttracking.cpp:120:11:120:16 | call to getenv | defaulttainttracking.cpp:124:12:124:12 | y | IR only |
3533
| defaulttainttracking.cpp:120:11:120:16 | call to getenv | defaulttainttracking.cpp:130:13:130:14 | & ... | IR only |
36-
| defaulttainttracking.cpp:120:11:120:16 | call to getenv | test_diff.cpp:2:11:2:13 | p#0 | IR only |
3734
| globals.cpp:13:15:13:20 | call to getenv | globals.cpp:13:5:13:11 | global1 | AST only |
3835
| globals.cpp:23:15:23:20 | call to getenv | globals.cpp:23:5:23:11 | global2 | AST only |
3936
| test_diff.cpp:104:12:104:15 | argv | test_diff.cpp:104:11:104:20 | (...) | IR only |

0 commit comments

Comments
 (0)