hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-26 09:15:12 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #14365 from MathiasVP/disable-flow-through-pointer-arith-for-size

Browse Source 
C++: Disable size-flow through pointer arithmetics in `cpp/invalid-pointer-deref`
This commit is contained in:
Mathias Vorreiter Pedersen
2023-10-06 10:14:31 +02:00
committed by GitHub
parent 97b3ebe385 843e9ad254
commit eb3f1967a5
4 changed files with 28 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ProductFlow.qll
View File

@@ -374,6 +374,8 @@ module ProductFlow {
predicate isBarrier(DataFlow::Node node, FlowState state) { Config::isBarrier1(node, state) }
predicate isBarrier(DataFlow::Node node) { Config::isBarrier1(node) }
predicate isBarrierOut(DataFlow::Node node) { Config::isBarrierOut1(node) }
predicate isAdditionalFlowStep(
@@ -408,6 +410,8 @@ module ProductFlow {
predicate isBarrier(DataFlow::Node node, FlowState state) { Config::isBarrier2(node, state) }
predicate isBarrier(DataFlow::Node node) { Config::isBarrier2(node) }
predicate isBarrierOut(DataFlow::Node node) { Config::isBarrierOut2(node) }
predicate isAdditionalFlowStep(

23
cpp/ql/lib/semmle/code/cpp/security/InvalidPointerDereference/AllocationToInvalidPointer.qll
View File

@@ -284,10 +284,33 @@ private module Config implements ProductFlow::StateConfigSig {
pointerAddInstructionHasBounds0(_, allocSink, sizeSink, sizeAddend)
}
private import semmle.code.cpp.ir.dataflow.internal.DataFlowPrivate
predicate isBarrier2(DataFlow::Node node, FlowState2 state) {
node = SizeBarrier::getABarrierNode(state)
}
predicate isBarrier2(DataFlow::Node node) {
// Block flow from `*p` to `*(p + n)` when `n` is not `0`. This removes
// false positives
// when tracking the size of the allocation as an element of an array such
// as:
// ```
// size_t* p = new size_t[n];
// ...
// p[0] = n;
// int i = p[1];
// p[i] = ...
// ```
// In the above case, this barrier blocks flow from the indirect node
// for `p` to `p[1]`.
exists(Operand operand, PointerAddInstruction add |
node.(IndirectOperand).hasOperandAndIndirectionIndex(operand, _) and
add.getLeftOperand() = operand and
add.getRight().(ConstantInstruction).getValue() != "0"
)
}
predicate isBarrierIn1(DataFlow::Node node) { isSourcePair(node, _, _, _) }
predicate isBarrierOut2(DataFlow::Node node) {

4
cpp/ql/test/query-tests/Security/CWE/CWE-193/InvalidPointerDeref.expected
View File

@@ -46,7 +46,6 @@ edges
| test.cpp:206:17:206:23 | ... + ... | test.cpp:213:5:213:13 | ... = ... |
| test.cpp:231:18:231:30 | new[] | test.cpp:232:3:232:20 | ... = ... |
| test.cpp:238:20:238:32 | new[] | test.cpp:239:5:239:22 | ... = ... |
| test.cpp:248:13:248:36 | call to realloc | test.cpp:254:9:254:16 | ... = ... |
| test.cpp:260:13:260:24 | new[] | test.cpp:261:14:261:21 | ... + ... |
| test.cpp:260:13:260:24 | new[] | test.cpp:261:14:261:21 | ... + ... |
| test.cpp:260:13:260:24 | new[] | test.cpp:264:13:264:14 | * ... |
@@ -215,8 +214,6 @@ nodes
| test.cpp:232:3:232:20 | ... = ... | semmle.label | ... = ... |
| test.cpp:238:20:238:32 | new[] | semmle.label | new[] |
| test.cpp:239:5:239:22 | ... = ... | semmle.label | ... = ... |
| test.cpp:248:13:248:36 | call to realloc | semmle.label | call to realloc |
| test.cpp:254:9:254:16 | ... = ... | semmle.label | ... = ... |
| test.cpp:260:13:260:24 | new[] | semmle.label | new[] |
| test.cpp:261:14:261:21 | ... + ... | semmle.label | ... + ... |
| test.cpp:261:14:261:21 | ... + ... | semmle.label | ... + ... |
@@ -322,7 +319,6 @@ subpaths
| test.cpp:213:5:213:13 | ... = ... | test.cpp:205:15:205:33 | call to malloc | test.cpp:213:5:213:13 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:205:15:205:33 | call to malloc | call to malloc | test.cpp:206:21:206:23 | len | len |
| test.cpp:232:3:232:20 | ... = ... | test.cpp:231:18:231:30 | new[] | test.cpp:232:3:232:20 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:231:18:231:30 | new[] | new[] | test.cpp:232:11:232:15 | index | index |
| test.cpp:239:5:239:22 | ... = ... | test.cpp:238:20:238:32 | new[] | test.cpp:239:5:239:22 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:238:20:238:32 | new[] | new[] | test.cpp:239:13:239:17 | index | index |
| test.cpp:254:9:254:16 | ... = ... | test.cpp:248:13:248:36 | call to realloc | test.cpp:254:9:254:16 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:248:13:248:36 | call to realloc | call to realloc | test.cpp:254:11:254:11 | i | i |
| test.cpp:264:13:264:14 | * ... | test.cpp:260:13:260:24 | new[] | test.cpp:264:13:264:14 | * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:260:13:260:24 | new[] | new[] | test.cpp:261:19:261:21 | len | len |
| test.cpp:274:5:274:10 | ... = ... | test.cpp:270:13:270:24 | new[] | test.cpp:274:5:274:10 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:270:13:270:24 | new[] | new[] | test.cpp:271:19:271:21 | len | len |
| test.cpp:358:14:358:26 | end_plus_one indirection | test.cpp:355:14:355:27 | new[] | test.cpp:358:14:358:26 | end_plus_one indirection | This read might be out of bounds, as the pointer might be equal to $@ + $@ + 1. | test.cpp:355:14:355:27 | new[] | new[] | test.cpp:356:20:356:23 | size | size |

2
cpp/ql/test/query-tests/Security/CWE/CWE-193/test.cpp
View File

@@ -251,7 +251,7 @@ void test17(unsigned *p, unsigned x, unsigned k) {
// The following access is okay because:
// n = 3*p[0] + k >= p[0] + k >= p[1] + k > p[1] = i
// (where p[0] denotes the original value for p[0])
p[i] = x; // $ alloc=L248 deref=L254 // GOOD [FALSE POSITIVE]
p[i] = x; // GOOD
}
}