diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll index f834259dc37..6470741d541 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll @@ -1520,6 +1520,25 @@ private module Cached { ) } + /** + * Holds if `operand.getDef() = instr`, but there exists a `StoreInstruction` that + * writes to an address that is equivalent to the value computed by `instr` in + * between `instr` and `operand`, and therefore there should not be flow from `*instr` + * to `*operand`. + */ + pragma[nomagic] + private predicate isStoredToBetween(Instruction instr, Operand operand) { + simpleOperandLocalFlowStep(pragma[only_bind_into](instr), pragma[only_bind_into](operand)) and + exists(StoreInstruction store, IRBlock block, int storeIndex, int instrIndex, int operandIndex | + store.getDestinationAddress() = instr and + block.getInstruction(storeIndex) = store and + block.getInstruction(instrIndex) = instr and + block.getInstruction(operandIndex) = operand.getUse() and + instrIndex < storeIndex and + storeIndex < operandIndex + ) + } + private predicate indirectionInstructionFlow( RawIndirectInstruction nodeFrom, IndirectOperand nodeTo ) { @@ -1529,7 +1548,8 @@ private module Cached { simpleOperandLocalFlowStep(pragma[only_bind_into](instr), pragma[only_bind_into](operand)) | hasOperandAndIndex(nodeTo, operand, pragma[only_bind_into](indirectionIndex)) and - hasInstructionAndIndex(nodeFrom, instr, pragma[only_bind_into](indirectionIndex)) + hasInstructionAndIndex(nodeFrom, instr, pragma[only_bind_into](indirectionIndex)) and + not isStoredToBetween(instr, operand) ) } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll index 60cb843b61f..4410e2e9e69 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll @@ -6,6 +6,7 @@ private import DataFlowImplCommon as DataFlowImplCommon private import DataFlowUtil private import semmle.code.cpp.models.interfaces.PointerWrapper private import DataFlowPrivate +private import semmle.code.cpp.ir.ValueNumbering /** * Holds if `operand` is an operand that is not used by the dataflow library. @@ -864,7 +865,7 @@ private module Cached { * to a specific address. */ private predicate isCertainAddress(Operand operand) { - operand.getDef() instanceof VariableAddressInstruction + valueNumberOfOperand(operand).getAnInstruction() instanceof VariableAddressInstruction or operand.getType() instanceof Cpp::ReferenceType } diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp index c5004157af1..c49d9092cd7 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp @@ -732,7 +732,7 @@ void test_does_not_write_source_to_dereference() { int x; does_not_write_source_to_dereference(&x); - sink(x); // $ ast,ir=733:7 SPURIOUS: ast,ir=726:11 + sink(x); // $ ast=733:7 ir SPURIOUS: ast=726:11 } void sometimes_calls_sink_eq(int x, int n) { diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp index 9810418a95e..2df0fc85bb6 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp @@ -134,7 +134,7 @@ void pointer_test() { sink(*p3); // $ ast,ir *p3 = 0; - sink(*p3); // $ SPURIOUS: ast,ir + sink(*p3); // $ SPURIOUS: ast } // --- return values ---