Skip to content

Commit ac4933a

Browse files
committed
C++: Ensure that the sink instruction occurs last in cpp/invalid-pointer-deref
This avoids some counter-intuitive paths where we would seemingly jump back to an earlier instruction, which might actually have been in bounds.
1 parent 41bd1ae commit ac4933a

File tree

3 files changed

+20
-299
lines changed

3 files changed

+20
-299
lines changed

cpp/ql/src/experimental/Security/CWE/CWE-193/InvalidPointerDeref.ql

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,22 @@ predicate isSinkImpl(
179179
pointerAddInstructionHasBounds(pai, sink1, sink2, delta)
180180
}
181181

182+
/**
183+
* Yields any instruction that is control-flow reachable from `instr`.
184+
*/
185+
Instruction getASuccessor(Instruction instr) {
186+
exists(IRBlock b, int instrIndex, int resultIndex |
187+
result.getBlock() = b and
188+
instr.getBlock() = b and
189+
b.getInstruction(instrIndex) = instr and
190+
b.getInstruction(resultIndex) = result
191+
|
192+
resultIndex >= instrIndex
193+
)
194+
or
195+
instr.getBlock().getASuccessor+() = result.getBlock()
196+
}
197+
182198
/**
183199
* Holds if `sink` is a sink for `InvalidPointerToDerefConfig` and `i` is a `StoreInstruction` that
184200
* writes to an address that non-strictly upper-bounds `sink`, or `i` is a `LoadInstruction` that
@@ -189,7 +205,8 @@ predicate isInvalidPointerDerefSink(DataFlow::Node sink, Instruction i, string o
189205
exists(AddressOperand addr |
190206
bounded1(addr.getDef(), sink.asInstruction(), delta) and
191207
delta >= 0 and
192-
i.getAnOperand() = addr
208+
i.getAnOperand() = addr and
209+
i = getASuccessor(sink.asInstruction())
193210
|
194211
i instanceof StoreInstruction and
195212
operation = "write"

0 commit comments

Comments
 (0)