Skip to content

Commit 8daa8d7

Browse files
MathiasVPMathiasVP
authored
Merge pull request github#13246 from MathiasVP/fix-pointer-pointee-conflation-2
C++: Fix more pointer/pointee conflation
2 parents e33f3a6 + b32d55a commit 8daa8d7

File tree

6 files changed

+21
-61
lines changed

6 files changed

+21
-61
lines changed

‎cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -210,8 +210,8 @@ class IndirectOperand extends Node {
210210
this.(RawIndirectOperand).getOperand() = operand and
211211
this.(RawIndirectOperand).getIndirectionIndex() = indirectionIndex
212212
or
213-
this.(OperandNode).getOperand() =
214-
Ssa::getIRRepresentationOfIndirectOperand(operand, indirectionIndex)
213+
nodeHasOperand(this, Ssa::getIRRepresentationOfIndirectOperand(operand, indirectionIndex),
214+
indirectionIndex - 1)
215215
}
216216

217217
/** Gets the underlying operand. */
@@ -250,8 +250,8 @@ class IndirectInstruction extends Node {
250250
this.(RawIndirectInstruction).getInstruction() = instr and
251251
this.(RawIndirectInstruction).getIndirectionIndex() = indirectionIndex
252252
or
253-
this.(InstructionNode).getInstruction() =
254-
Ssa::getIRRepresentationOfIndirectInstruction(instr, indirectionIndex)
253+
nodeHasInstruction(this, Ssa::getIRRepresentationOfIndirectInstruction(instr, indirectionIndex),
254+
indirectionIndex - 1)
255255
}
256256

257257
/** Gets the underlying instruction. */

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/InvalidPointerDeref.expected

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -654,30 +654,6 @@ edges
654654
| test.cpp:308:5:308:6 | xs | test.cpp:308:5:308:11 | access to array |
655655
| test.cpp:308:5:308:11 | access to array | test.cpp:308:5:308:29 | Store: ... = ... |
656656
| test.cpp:313:16:313:29 | new[] | test.cpp:314:17:314:18 | xs |
657-
| test.cpp:314:17:314:18 | xs | test.cpp:314:17:314:25 | ... + ... |
658-
| test.cpp:314:17:314:18 | xs | test.cpp:314:17:314:25 | ... + ... |
659-
| test.cpp:314:17:314:18 | xs | test.cpp:318:13:318:20 | * ... |
660-
| test.cpp:314:17:314:25 | ... + ... | test.cpp:318:14:318:20 | current |
661-
| test.cpp:314:17:314:25 | ... + ... | test.cpp:318:14:318:20 | current |
662-
| test.cpp:314:17:314:25 | ... + ... | test.cpp:320:13:320:20 | * ... |
663-
| test.cpp:314:17:314:25 | ... + ... | test.cpp:320:13:320:20 | * ... |
664-
| test.cpp:314:17:314:25 | ... + ... | test.cpp:320:14:320:20 | current |
665-
| test.cpp:314:17:314:25 | ... + ... | test.cpp:320:14:320:20 | current |
666-
| test.cpp:318:13:318:20 | * ... | test.cpp:318:14:318:20 | current |
667-
| test.cpp:318:13:318:20 | * ... | test.cpp:320:13:320:20 | * ... |
668-
| test.cpp:318:13:318:20 | * ... | test.cpp:320:14:320:20 | current |
669-
| test.cpp:318:14:318:20 | current | test.cpp:314:17:314:25 | Store: ... + ... |
670-
| test.cpp:318:14:318:20 | current | test.cpp:318:13:318:20 | Load: * ... |
671-
| test.cpp:318:14:318:20 | current | test.cpp:320:10:320:21 | Store: -- ... |
672-
| test.cpp:318:14:318:20 | current | test.cpp:320:12:320:21 | Load: (...) |
673-
| test.cpp:320:13:320:20 | * ... | test.cpp:314:17:314:25 | Store: ... + ... |
674-
| test.cpp:320:13:320:20 | * ... | test.cpp:318:13:318:20 | Load: * ... |
675-
| test.cpp:320:13:320:20 | * ... | test.cpp:320:10:320:21 | Store: -- ... |
676-
| test.cpp:320:13:320:20 | * ... | test.cpp:320:12:320:21 | Load: (...) |
677-
| test.cpp:320:14:320:20 | current | test.cpp:314:17:314:25 | Store: ... + ... |
678-
| test.cpp:320:14:320:20 | current | test.cpp:318:13:318:20 | Load: * ... |
679-
| test.cpp:320:14:320:20 | current | test.cpp:320:10:320:21 | Store: -- ... |
680-
| test.cpp:320:14:320:20 | current | test.cpp:320:12:320:21 | Load: (...) |
681657
subpaths
682658
#select
683659
| test.cpp:6:14:6:15 | Load: * ... | test.cpp:4:15:4:20 | call to malloc | test.cpp:6:14:6:15 | Load: * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:4:15:4:20 | call to malloc | call to malloc | test.cpp:5:19:5:22 | size | size |
@@ -703,7 +679,3 @@ subpaths
703679
| test.cpp:264:13:264:14 | Load: * ... | test.cpp:260:13:260:24 | new[] | test.cpp:264:13:264:14 | Load: * ... | 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 |
704680
| test.cpp:274:5:274:10 | Store: ... = ... | test.cpp:270:13:270:24 | new[] | test.cpp:274:5:274:10 | Store: ... = ... | 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 |
705681
| test.cpp:308:5:308:29 | Store: ... = ... | test.cpp:304:15:304:26 | new[] | test.cpp:308:5:308:29 | Store: ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:304:15:304:26 | new[] | new[] | test.cpp:308:8:308:10 | ... + ... | ... + ... |
706-
| test.cpp:314:17:314:25 | Store: ... + ... | test.cpp:313:16:313:29 | new[] | test.cpp:314:17:314:25 | Store: ... + ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:313:16:313:29 | new[] | new[] | test.cpp:314:22:314:25 | size | size |
707-
| test.cpp:318:13:318:20 | Load: * ... | test.cpp:313:16:313:29 | new[] | test.cpp:318:13:318:20 | Load: * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:313:16:313:29 | new[] | new[] | test.cpp:314:22:314:25 | size | size |
708-
| test.cpp:320:10:320:21 | Store: -- ... | test.cpp:313:16:313:29 | new[] | test.cpp:320:10:320:21 | Store: -- ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:313:16:313:29 | new[] | new[] | test.cpp:314:22:314:25 | size | size |
709-
| test.cpp:320:12:320:21 | Load: (...) | test.cpp:313:16:313:29 | new[] | test.cpp:320:12:320:21 | Load: (...) | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:313:16:313:29 | new[] | new[] | test.cpp:314:22:314:25 | size | size |

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/test.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -311,13 +311,13 @@ void test21() {
311311

312312
void test22(unsigned size, int val) {
313313
char *xs = new char[size];
314-
char *end = xs + size; // GOOD [FALSE POSITIVE]
314+
char *end = xs + size; // GOOD
315315
char **current = &end;
316316
do
317317
{
318-
if( *current - xs < 1 ) // GOOD [FALSE POSITIVE]
318+
if( *current - xs < 1 ) // GOOD
319319
return;
320-
*--(*current) = 0; // GOOD [FALSE POSITIVE]
320+
*--(*current) = 0; // GOOD
321321
val >>= 8;
322322
}
323323
while( val > 0 );

‎cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8090,20 +8090,20 @@
80908090
| vector.cpp:520:25:520:31 | call to vector | vector.cpp:523:8:523:9 | vs | |
80918091
| vector.cpp:520:25:520:31 | call to vector | vector.cpp:524:8:524:9 | vs | |
80928092
| vector.cpp:520:25:520:31 | call to vector | vector.cpp:526:8:526:9 | vs | |
8093-
| vector.cpp:520:25:520:31 | call to vector | vector.cpp:539:8:539:9 | vs | |
8094-
| vector.cpp:520:25:520:31 | call to vector | vector.cpp:540:2:540:2 | vs | |
8093+
| vector.cpp:520:25:520:31 | call to vector | vector.cpp:532:8:532:9 | vs | |
8094+
| vector.cpp:520:25:520:31 | call to vector | vector.cpp:533:2:533:2 | vs | |
80958095
| vector.cpp:520:30:520:30 | 0 | vector.cpp:520:25:520:31 | call to vector | TAINT |
80968096
| vector.cpp:523:8:523:9 | ref arg vs | vector.cpp:524:8:524:9 | vs | |
80978097
| vector.cpp:523:8:523:9 | ref arg vs | vector.cpp:526:8:526:9 | vs | |
8098-
| vector.cpp:523:8:523:9 | ref arg vs | vector.cpp:539:8:539:9 | vs | |
8099-
| vector.cpp:523:8:523:9 | ref arg vs | vector.cpp:540:2:540:2 | vs | |
8098+
| vector.cpp:523:8:523:9 | ref arg vs | vector.cpp:532:8:532:9 | vs | |
8099+
| vector.cpp:523:8:523:9 | ref arg vs | vector.cpp:533:2:533:2 | vs | |
81008100
| vector.cpp:523:8:523:9 | vs | vector.cpp:523:10:523:10 | call to operator[] | TAINT |
81018101
| vector.cpp:524:8:524:9 | ref arg vs | vector.cpp:526:8:526:9 | vs | |
8102-
| vector.cpp:524:8:524:9 | ref arg vs | vector.cpp:539:8:539:9 | vs | |
8103-
| vector.cpp:524:8:524:9 | ref arg vs | vector.cpp:540:2:540:2 | vs | |
8102+
| vector.cpp:524:8:524:9 | ref arg vs | vector.cpp:532:8:532:9 | vs | |
8103+
| vector.cpp:524:8:524:9 | ref arg vs | vector.cpp:533:2:533:2 | vs | |
81048104
| vector.cpp:524:8:524:9 | vs | vector.cpp:524:10:524:10 | call to operator[] | TAINT |
8105-
| vector.cpp:526:8:526:9 | ref arg vs | vector.cpp:539:8:539:9 | vs | |
8106-
| vector.cpp:526:8:526:9 | ref arg vs | vector.cpp:540:2:540:2 | vs | |
8105+
| vector.cpp:526:8:526:9 | ref arg vs | vector.cpp:532:8:532:9 | vs | |
8106+
| vector.cpp:526:8:526:9 | ref arg vs | vector.cpp:533:2:533:2 | vs | |
81078107
| vector.cpp:526:8:526:9 | vs | vector.cpp:526:11:526:15 | call to begin | TAINT |
81088108
| vector.cpp:526:11:526:15 | call to begin | vector.cpp:526:3:526:17 | ... = ... | |
81098109
| vector.cpp:526:11:526:15 | call to begin | vector.cpp:527:9:527:10 | it | |
@@ -8128,5 +8128,5 @@
81288128
| vector.cpp:530:3:530:4 | ref arg it | vector.cpp:531:9:531:10 | it | |
81298129
| vector.cpp:530:9:530:14 | call to source | vector.cpp:530:3:530:4 | ref arg it | TAINT |
81308130
| vector.cpp:531:9:531:10 | it | vector.cpp:531:8:531:8 | call to operator* | TAINT |
8131-
| vector.cpp:539:8:539:9 | ref arg vs | vector.cpp:540:2:540:2 | vs | |
8132-
| vector.cpp:539:8:539:9 | vs | vector.cpp:539:10:539:10 | call to operator[] | TAINT |
8131+
| vector.cpp:532:8:532:9 | ref arg vs | vector.cpp:533:2:533:2 | vs | |
8132+
| vector.cpp:532:8:532:9 | vs | vector.cpp:532:10:532:10 | call to operator[] | TAINT |

‎cpp/ql/test/library-tests/dataflow/taint-tests/vector.cpp

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -523,19 +523,12 @@ void test_vector_iterator() {
523523
sink(vs[1]);
524524
sink(vs[source()]); // $ MISSING: ast,ir
525525

526-
it = vs.begin(); // (1)
526+
it = vs.begin();
527527
sink(*it);
528528
it += 1;
529529
sink(*it);
530-
it += source(); // (2)
531-
sink(*it); // $ ast,ir // (3)
532-
// This FP happens because of the following flows:
533-
// 1. There's a write to the iterator at (2)
534-
// 2. This write propagates to `it` on the next line at (3)
535-
// 3. There's a taint step from `it` to `*it` at (3)
536-
// 4. The `*it` is seen as a use of `vs` because of (1).
537-
// 5. There's use-use flow from `*it` at (3) (which is a use of `vs`) to `vs` at (4)
538-
// 6. There's a taint step from vs to vs[1]
539-
sink(vs[1]); // $ SPURIOUS: ir // (4)
530+
it += source();
531+
sink(*it); // $ ast,ir
532+
sink(vs[1]); // clean
540533
}
541534
}

‎cpp/ql/test/query-tests/Critical/MemoryFreed/UseAfterFree.expected

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@ edges
2323
| test_free.cpp:239:14:239:15 | * ... | test_free.cpp:241:9:241:10 | * ... |
2424
| test_free.cpp:239:14:239:15 | * ... | test_free.cpp:241:9:241:10 | * ... |
2525
| test_free.cpp:239:14:239:15 | * ... | test_free.cpp:241:9:241:10 | * ... |
26-
| test_free.cpp:239:14:239:15 | * ... | test_free.cpp:241:10:241:10 | b |
27-
| test_free.cpp:239:14:239:15 | * ... | test_free.cpp:241:10:241:10 | b |
2826
| test_free.cpp:245:10:245:11 | * ... | test_free.cpp:246:9:246:10 | * ... |
2927
| test_free.cpp:245:10:245:11 | * ... | test_free.cpp:246:9:246:10 | * ... |
3028
| test_free.cpp:245:10:245:11 | * ... | test_free.cpp:246:9:246:10 | * ... |
@@ -61,7 +59,6 @@ nodes
6159
| test_free.cpp:239:14:239:15 | * ... | semmle.label | * ... |
6260
| test_free.cpp:241:9:241:10 | * ... | semmle.label | * ... |
6361
| test_free.cpp:241:9:241:10 | * ... | semmle.label | * ... |
64-
| test_free.cpp:241:10:241:10 | b | semmle.label | b |
6562
| test_free.cpp:245:10:245:11 | * ... | semmle.label | * ... |
6663
| test_free.cpp:245:10:245:11 | * ... | semmle.label | * ... |
6764
| test_free.cpp:246:9:246:10 | * ... | semmle.label | * ... |
@@ -92,8 +89,6 @@ subpaths
9289
| test_free.cpp:241:9:241:10 | * ... | test_free.cpp:239:14:239:15 | * ... | test_free.cpp:241:9:241:10 | * ... | Memory may have been previously freed by $@. | test_free.cpp:239:9:239:12 | call to free | call to free |
9390
| test_free.cpp:241:9:241:10 | * ... | test_free.cpp:239:14:239:15 | * ... | test_free.cpp:241:9:241:10 | * ... | Memory may have been previously freed by $@. | test_free.cpp:239:9:239:12 | call to free | call to free |
9491
| test_free.cpp:241:9:241:10 | * ... | test_free.cpp:239:14:239:15 | * ... | test_free.cpp:241:9:241:10 | * ... | Memory may have been previously freed by $@. | test_free.cpp:239:9:239:12 | call to free | call to free |
95-
| test_free.cpp:241:10:241:10 | b | test_free.cpp:239:14:239:15 | * ... | test_free.cpp:241:10:241:10 | b | Memory may have been previously freed by $@. | test_free.cpp:239:9:239:12 | call to free | call to free |
96-
| test_free.cpp:241:10:241:10 | b | test_free.cpp:239:14:239:15 | * ... | test_free.cpp:241:10:241:10 | b | Memory may have been previously freed by $@. | test_free.cpp:239:9:239:12 | call to free | call to free |
9792
| test_free.cpp:246:9:246:10 | * ... | test_free.cpp:245:10:245:11 | * ... | test_free.cpp:246:9:246:10 | * ... | Memory may have been previously freed by $@. | test_free.cpp:245:5:245:8 | call to free | call to free |
9893
| test_free.cpp:246:9:246:10 | * ... | test_free.cpp:245:10:245:11 | * ... | test_free.cpp:246:9:246:10 | * ... | Memory may have been previously freed by $@. | test_free.cpp:245:5:245:8 | call to free | call to free |
9994
| test_free.cpp:246:9:246:10 | * ... | test_free.cpp:245:10:245:11 | * ... | test_free.cpp:246:9:246:10 | * ... | Memory may have been previously freed by $@. | test_free.cpp:245:5:245:8 | call to free | call to free |

0 commit comments

Comments
 (0)