hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-24 04:36:35 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #14512 from MathiasVP/fix-size-in-invalid-ptr-deref

Browse Source 
C++: Fix size deduction in `cpp/invalid-pointer-deref`
This commit is contained in:
Dave Bartolomeo
2023-10-16 11:22:41 -04:00
committed by GitHub
parent 25c416ec8a d8a049f5cc
commit e4e472ee74
3 changed files with 39 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -60,17 +60,31 @@ private import semmle.code.cpp.rangeanalysis.new.RangeAnalysisUtil
private VariableAccess getAVariableAccess(Expr e) { e.getAChild*() = result }
/**
* Gets a (sub)expression that may be the result of evaluating `size`.
*
* For example, `getASizeCandidate(a ? b : c)` gives `a ? b : c`, `b` and `c`.
*/
bindingset[size]
pragma[inline_late]
private Expr getASizeCandidate(Expr size) {
result = size
or
result = [size.(ConditionalExpr).getThen(), size.(ConditionalExpr).getElse()]
}
/**
* Holds if the `(n, state)` pair represents the source of flow for the size
* expression associated with `alloc`.
*/
predicate hasSize(HeuristicAllocationExpr alloc, DataFlow::Node n, int state) {
exists(VariableAccess va, Expr size, int delta |
exists(VariableAccess va, Expr size, int delta, Expr s |
size = alloc.getSizeExpr() and
s = getASizeCandidate(size) and
// Get the unique variable in a size expression like `x` in `malloc(x + 1)`.
va = unique( | | getAVariableAccess(size)) and
va = unique( | | getAVariableAccess(s)) and
// Compute `delta` as the constant difference between `x` and `x + 1`.
bounded1(any(Instruction instr | instr.getUnconvertedResultExpression() = size),
bounded1(any(Instruction instr | instr.getUnconvertedResultExpression() = s),
any(LoadInstruction load | load.getUnconvertedResultExpression() = va), delta) and
n.asExpr() = va and
state = delta

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

@@ -181,6 +181,12 @@ edges
| test.cpp:833:37:833:39 | end | test.cpp:815:52:815:54 | end |
| test.cpp:841:18:841:35 | call to malloc | test.cpp:842:3:842:20 | ... = ... |
| test.cpp:848:20:848:37 | call to malloc | test.cpp:849:5:849:22 | ... = ... |
| test.cpp:856:12:856:35 | call to malloc | test.cpp:857:16:857:29 | ... + ... |
| test.cpp:856:12:856:35 | call to malloc | test.cpp:857:16:857:29 | ... + ... |
| test.cpp:856:12:856:35 | call to malloc | test.cpp:860:5:860:11 | ... = ... |
| test.cpp:857:16:857:29 | ... + ... | test.cpp:857:16:857:29 | ... + ... |
| test.cpp:857:16:857:29 | ... + ... | test.cpp:860:5:860:11 | ... = ... |
| test.cpp:857:16:857:29 | ... + ... | test.cpp:860:5:860:11 | ... = ... |
nodes
| test.cpp:4:15:4:33 | call to malloc | semmle.label | call to malloc |
| test.cpp:5:15:5:22 | ... + ... | semmle.label | ... + ... |
@@ -307,6 +313,10 @@ nodes
| test.cpp:842:3:842:20 | ... = ... | semmle.label | ... = ... |
| test.cpp:848:20:848:37 | call to malloc | semmle.label | call to malloc |
| test.cpp:849:5:849:22 | ... = ... | semmle.label | ... = ... |
| test.cpp:856:12:856:35 | call to malloc | semmle.label | call to malloc |
| test.cpp:857:16:857:29 | ... + ... | semmle.label | ... + ... |
| test.cpp:857:16:857:29 | ... + ... | semmle.label | ... + ... |
| test.cpp:860:5:860:11 | ... = ... | semmle.label | ... = ... |
subpaths
#select
| test.cpp:6:14:6:15 | * ... | test.cpp:4:15:4:33 | call to malloc | test.cpp:6:14:6:15 | * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:4:15:4:33 | call to malloc | call to malloc | test.cpp:5:19:5:22 | size | size |
@@ -344,3 +354,4 @@ subpaths
| test.cpp:821:7:821:12 | ... = ... | test.cpp:793:14:793:32 | call to malloc | test.cpp:821:7:821:12 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:793:14:793:32 | call to malloc | call to malloc | test.cpp:794:21:794:24 | size | size |
| test.cpp:842:3:842:20 | ... = ... | test.cpp:841:18:841:35 | call to malloc | test.cpp:842:3:842:20 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:841:18:841:35 | call to malloc | call to malloc | test.cpp:842:11:842:15 | index | index |
| test.cpp:849:5:849:22 | ... = ... | test.cpp:848:20:848:37 | call to malloc | test.cpp:849:5:849:22 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:848:20:848:37 | call to malloc | call to malloc | test.cpp:849:13:849:17 | index | index |
| test.cpp:860:5:860:11 | ... = ... | test.cpp:856:12:856:35 | call to malloc | test.cpp:860:5:860:11 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:856:12:856:35 | call to malloc | call to malloc | test.cpp:857:21:857:28 | ... + ... | ... + ... |

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

@@ -848,4 +848,15 @@ void test16_with_malloc(size_t index) {
int* newname = (int*)malloc(size);
newname[index] = 0; // $ SPURIOUS: alloc=L848 deref=L849 // GOOD [FALSE POSITIVE]
}
}
# define MyMalloc(size) malloc(((size) == 0 ? 1 : (size)))
void test_regression(size_t size) {
int* p = (int*)MyMalloc(size + 1);
int* chend = p + (size + 1); // $ alloc=L856+1
if(p <= chend) {
*p = 42; // $ deref=L860 // BAD
}
}