diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll index 32cdc7ddfcb..081cf513aae 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll @@ -265,7 +265,21 @@ private predicate simpleInstructionLocalFlowStep(Instruction iFrom, Instruction iTo.(PhiInstruction).getAnOperand().getDef() = iFrom or // Treat all conversions as flow, even conversions between different numeric types. iTo.(ConvertInstruction).getUnary() = iFrom or - iTo.(InheritanceConversionInstruction).getUnary() = iFrom + iTo.(InheritanceConversionInstruction).getUnary() = iFrom or + // A chi instruction represents a point where a new value (the _partial_ + // operand) may overwrite an old value (the _total_ operand), but the alias + // analysis couldn't determine that it surely will overwrite every bit of it or + // that it surely will overwrite no bit of it. + // + // By allowing flow through the total operand, we ensure that flow is not lost + // due to shortcomings of the alias analysis. We may get false flow in cases + // where the data is indeed overwritten. + // + // Allowing flow through the partial operand would be more noisy, especially + // for variables that have escaped: for soundness, the IR has to assume that + // every write to an unknown address can affect every escaped variable, and + // this assumption shows up as data flowing through partial chi operands. + iTo.getAnOperand().(ChiTotalOperand).getDef() = iFrom } /** diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected index f0725b21050..0cc75115164 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected @@ -22,6 +22,5 @@ | taint.cpp:250:8:250:8 | taint.cpp:223:10:223:15 | AST only | | taint.cpp:256:8:256:8 | taint.cpp:223:10:223:15 | AST only | | taint.cpp:261:7:261:7 | taint.cpp:258:7:258:12 | AST only | -| taint.cpp:350:7:350:7 | taint.cpp:330:6:330:11 | AST only | | taint.cpp:351:7:351:7 | taint.cpp:330:6:330:11 | AST only | | taint.cpp:352:7:352:7 | taint.cpp:330:6:330:11 | AST only | diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.expected index 3336100fd48..81195e197c1 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.expected @@ -14,3 +14,4 @@ | taint.cpp:290:7:290:7 | x | taint.cpp:275:6:275:11 | call to source | | taint.cpp:291:7:291:7 | y | taint.cpp:275:6:275:11 | call to source | | taint.cpp:337:7:337:7 | t | taint.cpp:330:6:330:11 | call to source | +| taint.cpp:350:7:350:7 | t | taint.cpp:330:6:330:11 | call to source |