From d0849311a69cc6e6decf45056c14b43bfc3f8e20 Mon Sep 17 00:00:00 2001 From: Alex Eyers-Taylor Date: Wed, 20 Sep 2023 18:31:55 +0100 Subject: [PATCH] CPP: Fix use after free FPs by elimnatiing freeing nodes rather than freeing expressions. --- cpp/ql/src/Critical/UseAfterFree.ql | 4 ++-- .../query-tests/Critical/MemoryFreed/UseAfterFree.expected | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/cpp/ql/src/Critical/UseAfterFree.ql b/cpp/ql/src/Critical/UseAfterFree.ql index a4d1ee7be2f0..615995448441 100644 --- a/cpp/ql/src/Critical/UseAfterFree.ql +++ b/cpp/ql/src/Critical/UseAfterFree.ql @@ -31,7 +31,7 @@ private predicate externalCallNeverDereferences(FormattingFunctionCall call, int predicate isUse0(DataFlow::Node n, Expr e) { e = n.asExpr() and - not isFree(_, e, _) and + not isFree(n, _, _) and ( e = any(PointerDereferenceExpr pde).getOperand() or @@ -43,7 +43,7 @@ predicate isUse0(DataFlow::Node n, Expr e) { or // Assume any function without a body will dereference the pointer exists(int i, Call call, Function f | - n.asExpr() = call.getArgument(i) and + e = call.getArgument(i) and f = call.getTarget() and not f.hasEntryPoint() and // Exclude known functions we know won't dereference the pointer. diff --git a/cpp/ql/test/query-tests/Critical/MemoryFreed/UseAfterFree.expected b/cpp/ql/test/query-tests/Critical/MemoryFreed/UseAfterFree.expected index e89e2227cea9..b4ac51dc72e9 100644 --- a/cpp/ql/test/query-tests/Critical/MemoryFreed/UseAfterFree.expected +++ b/cpp/ql/test/query-tests/Critical/MemoryFreed/UseAfterFree.expected @@ -13,7 +13,6 @@ edges | test_free.cpp:239:14:239:15 | * ... | test_free.cpp:241:9:241:10 | * ... | | test_free.cpp:245:10:245:11 | * ... | test_free.cpp:246:9:246:10 | * ... | nodes -| test.cpp:205:7:205:11 | ... = ... | semmle.label | ... = ... | | test_free.cpp:11:10:11:10 | a | semmle.label | a | | test_free.cpp:12:5:12:5 | a | semmle.label | a | | test_free.cpp:13:5:13:6 | * ... | semmle.label | * ... | @@ -41,7 +40,6 @@ nodes | test_free.cpp:246:9:246:10 | * ... | semmle.label | * ... | subpaths #select -| test.cpp:205:7:205:11 | ... = ... | test.cpp:205:7:205:11 | ... = ... | test.cpp:205:7:205:11 | ... = ... | Memory may have been previously freed by $@. | test.cpp:205:2:205:5 | call to free | call to free | | test_free.cpp:12:5:12:5 | a | test_free.cpp:11:10:11:10 | a | test_free.cpp:12:5:12:5 | a | Memory may have been previously freed by $@. | test_free.cpp:11:5:11:8 | call to free | call to free | | test_free.cpp:13:5:13:6 | * ... | test_free.cpp:11:10:11:10 | a | test_free.cpp:13:5:13:6 | * ... | Memory may have been previously freed by $@. | test_free.cpp:11:5:11:8 | call to free | call to free | | test_free.cpp:45:5:45:5 | a | test_free.cpp:42:27:42:27 | a | test_free.cpp:45:5:45:5 | a | Memory may have been previously freed by $@. | test_free.cpp:42:22:42:25 | call to free | call to free |