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

Merge pull request #11823 from geoffw0/heuristicalloc

Browse Source 
C++: Use HeuristicAllocationExpr in more queries
This commit is contained in:
Geoffrey White
2023-01-06 13:13:14 +00:00
committed by GitHub
parent f5e5f6dfd1 bb451f3911
commit f3914ffe25
18 changed files with 124 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
cpp/ql/lib/semmle/code/cpp/models/interfaces/Allocation.qll
View File

@@ -133,13 +133,15 @@ abstract class HeuristicAllocationExpr extends Expr {
/**
* Gets a constant multiplier for the allocation size given by `getSizeExpr`,
* in bytes.
* in bytes. This predicate should be used with caution as it can be
* inaccurate for allocations identified using heuristics.
*/
int getSizeMult() { none() }
/**
* Gets the size of this allocation in bytes, if it is a fixed size and that
* size can be determined.
* size can be determined. This predicate should be used with caution as it
* can be inaccurate for allocations identified using heuristics.
*/
int getSizeBytes() { none() }

2
cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql
View File

@@ -21,7 +21,7 @@ import semmle.code.cpp.models.interfaces.ArrayFunction
import semmle.code.cpp.models.interfaces.Allocation
import semmle.code.cpp.commons.NullTermination
predicate terminationProblem(AllocationExpr malloc, string msg) {
predicate terminationProblem(HeuristicAllocationExpr malloc, string msg) {
// malloc(strlen(...))
exists(StrlenCall strlen | DataFlow::localExprFlow(strlen, malloc.getSizeExpr())) and
// flows to a call that implies this is a null-terminated string

7
cpp/ql/src/Security/CWE/CWE-190/TaintedAllocationSize.ql
View File

@@ -25,9 +25,8 @@ import DataFlow::PathGraph
* Holds if `alloc` is an allocation, and `tainted` is a child of it that is a
* taint sink.
*/
predicate allocSink(Expr alloc, DataFlow::Node sink) {
predicate allocSink(HeuristicAllocationExpr alloc, DataFlow::Node sink) {
exists(Expr e | e = sink.asConvertedExpr() |
isAllocationExpr(alloc) and
e = alloc.getAChild() and
e.getUnspecifiedType() instanceof IntegralType
)
@@ -89,6 +88,10 @@ class TaintedAllocationSizeConfiguration extends TaintTracking::Configuration {
readsVariable(access.getDef(), checkedVar) and
nodeIsBarrierEqualityCandidate(node, access, checkedVar)
)
or
// block flow to inside of identified allocation functions (this flow leads
// to duplicate results)
any(HeuristicAllocationFunction f).getAParameter() = node.asParameter()
}
}

4
cpp/ql/src/change-notes/2023-01-05-heuristic-allocations.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The `cpp/no-space-for-terminator` and `cpp/uncontrolled-allocation-size` queries have been enhanced with heuristic detection of allocations. These queries now find more results.

2
cpp/ql/src/experimental/Security/CWE/CWE-190/AllocMultiplicationOverflow.ql
View File

@@ -29,7 +29,7 @@ class MultToAllocConfig extends DataFlow::Configuration {
override predicate isSink(DataFlow::Node node) {
// something that affects an allocation size
node.asExpr() = any(AllocationExpr ae).getSizeExpr().getAChild*()
node.asExpr() = any(HeuristicAllocationExpr ae).getSizeExpr().getAChild*()
}
}

10
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-190/AllocMultiplicationOverflow/AllocMultiplicationOverflow.expected
View File

@@ -1,5 +1,7 @@
edges
| test.cpp:22:17:22:21 | ... * ... | test.cpp:23:33:23:37 | size1 |
| test.cpp:37:24:37:27 | size | test.cpp:37:46:37:49 | size |
| test.cpp:45:36:45:40 | ... * ... | test.cpp:37:24:37:27 | size |
nodes
| test.cpp:13:33:13:37 | ... * ... | semmle.label | ... * ... |
| test.cpp:15:31:15:35 | ... * ... | semmle.label | ... * ... |
@@ -8,6 +10,11 @@ nodes
| test.cpp:23:33:23:37 | size1 | semmle.label | size1 |
| test.cpp:30:27:30:31 | ... * ... | semmle.label | ... * ... |
| test.cpp:31:27:31:31 | ... * ... | semmle.label | ... * ... |
| test.cpp:37:24:37:27 | size | semmle.label | size |
| test.cpp:37:46:37:49 | size | semmle.label | size |
| test.cpp:45:36:45:40 | ... * ... | semmle.label | ... * ... |
| test.cpp:45:36:45:40 | ... * ... | semmle.label | ... * ... |
| test.cpp:46:36:46:40 | ... * ... | semmle.label | ... * ... |
subpaths
#select
| test.cpp:13:33:13:37 | ... * ... | test.cpp:13:33:13:37 | ... * ... | test.cpp:13:33:13:37 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:13:33:13:37 | ... * ... | multiplication |
@@ -16,3 +23,6 @@ subpaths
| test.cpp:23:33:23:37 | size1 | test.cpp:22:17:22:21 | ... * ... | test.cpp:23:33:23:37 | size1 | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:22:17:22:21 | ... * ... | multiplication |
| test.cpp:30:27:30:31 | ... * ... | test.cpp:30:27:30:31 | ... * ... | test.cpp:30:27:30:31 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:30:27:30:31 | ... * ... | multiplication |
| test.cpp:31:27:31:31 | ... * ... | test.cpp:31:27:31:31 | ... * ... | test.cpp:31:27:31:31 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:31:27:31:31 | ... * ... | multiplication |
| test.cpp:37:46:37:49 | size | test.cpp:45:36:45:40 | ... * ... | test.cpp:37:46:37:49 | size | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:45:36:45:40 | ... * ... | multiplication |
| test.cpp:45:36:45:40 | ... * ... | test.cpp:45:36:45:40 | ... * ... | test.cpp:45:36:45:40 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:45:36:45:40 | ... * ... | multiplication |
| test.cpp:46:36:46:40 | ... * ... | test.cpp:46:36:46:40 | ... * ... | test.cpp:46:36:46:40 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:46:36:46:40 | ... * ... | multiplication |

15
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-190/AllocMultiplicationOverflow/test.cpp
View File

@@ -30,3 +30,18 @@ void test()
char *buffer8 = new char[x * y]; // BAD
char *buffer9 = new char[x * x]; // BAD
}
// --- custom allocators ---
void *MyMalloc1(size_t size) { return malloc(size); } // [additional detection here]
void *MyMalloc2(size_t size);
void customAllocatorTests()
{
int x = getAnInt();
int y = getAnInt();
char *buffer1 = (char *)MyMalloc1(x * y); // BAD
char *buffer2 = (char *)MyMalloc2(x * y); // BAD
}

2
cpp/ql/test/query-tests/Critical/OverflowCalculated/NoSpaceForZeroTerminator.expected
View File

@@ -7,3 +7,5 @@
| tests3.cpp:25:21:25:31 | call to malloc | This allocation does not include space to null-terminate the string. |
| tests3.cpp:30:21:30:31 | call to malloc | This allocation does not include space to null-terminate the string. |
| tests3.cpp:53:17:53:44 | new[] | This allocation does not include space to null-terminate the string. |
| tests3.cpp:81:20:81:28 | call to MyMalloc1 | This allocation does not include space to null-terminate the string. |
| tests3.cpp:84:20:84:28 | call to MyMalloc2 | This allocation does not include space to null-terminate the string. |

20
cpp/ql/test/query-tests/Critical/OverflowCalculated/tests3.cpp
View File

@@ -1,4 +1,4 @@
// tests1.cpp
// tests3.cpp
typedef unsigned int size_t;
@@ -66,3 +66,21 @@ void test3c()
delete buffer;
}
// --- custom allocators ---
void *MyMalloc1(size_t size) { return std::malloc(size); }
void *MyMalloc2(size_t size);
void tests4()
{
const char *str4 = "1234";
char *buffer1 = 0;
char *buffer2 = 0;
buffer1 = (char *)MyMalloc1(strlen(str4)); // BAD
strcpy(buffer1, str4);
buffer2 = (char *)MyMalloc2(strlen(str4)); // BAD
strcpy(buffer2, str4);
}

11
cpp/ql/test/query-tests/Critical/SizeCheck/test.c
View File

@@ -58,3 +58,14 @@ void test_union() {
MyUnion *a = malloc(sizeof(MyUnion)); // GOOD
MyUnion *b = malloc(sizeof(MyStruct)); // BAD (too small)
}
// --- custom allocators ---
void *MyMalloc1(size_t size) { return malloc(size); }
void *MyMalloc2(size_t size);
void customAllocatorTests()
{
float *fptr1 = MyMalloc1(3); // BAD (too small) [NOT DETECTED]
float *fptr2 = MyMalloc2(3); // BAD (too small) [NOT DETECTED]
}

10
cpp/ql/test/query-tests/Critical/SizeCheck/test2.c
View File

@@ -43,5 +43,13 @@ void good1(void) {
free(dptr);
}
// --- custom allocators ---
void *MyMalloc1(size_t size) { return malloc(size); }
void *MyMalloc2(size_t size);
void customAllocatorTests()
{
double *dptr1 = MyMalloc1(33); // BAD -- Not a multiple of sizeof(double) [NOT DETECTED]
double *dptr2 = MyMalloc2(33); // BAD -- Not a multiple of sizeof(double) [NOT DETECTED]
}

2
cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/NoSpaceForZeroTerminator.expected → cpp/ql/test/query-tests/Security/CWE/CWE-131/NoSpaceForZeroTerminator/NoSpaceForZeroTerminator.expected
View File

@@ -1,5 +1,7 @@
| test2.cpp:64:34:64:39 | call to calloc | This allocation does not include space to null-terminate the string. |
| test2.cpp:71:28:71:34 | call to realloc | This allocation does not include space to null-terminate the string. |
| test2.cpp:84:27:84:35 | call to MyMalloc1 | This allocation does not include space to null-terminate the string. |
| test2.cpp:89:27:89:35 | call to MyMalloc2 | This allocation does not include space to null-terminate the string. |
| test.c:16:20:16:25 | call to malloc | This allocation does not include space to null-terminate the string. |
| test.c:32:20:32:25 | call to malloc | This allocation does not include space to null-terminate the string. |
| test.c:49:20:49:25 | call to malloc | This allocation does not include space to null-terminate the string. |

0
cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/NoSpaceForZeroTerminator.qlref → cpp/ql/test/query-tests/Security/CWE/CWE-131/NoSpaceForZeroTerminator/NoSpaceForZeroTerminator.qlref
View File

0
cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/test.c → cpp/ql/test/query-tests/Security/CWE/CWE-131/NoSpaceForZeroTerminator/test.c
View File

0
cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/test.cpp → cpp/ql/test/query-tests/Security/CWE/CWE-131/NoSpaceForZeroTerminator/test.cpp
View File

18
cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/test2.cpp → cpp/ql/test/query-tests/Security/CWE/CWE-131/NoSpaceForZeroTerminator/test2.cpp
View File

@@ -72,3 +72,21 @@ void bad4(char *str) {
strcpy(buffer, str);
free(buffer);
}
// --- custom allocators ---
void *MyMalloc1(size_t size) { return malloc(size); }
void *MyMalloc2(size_t size);
void customAllocatorTests(char *str)
{
{
char *buffer1 = (char *)MyMalloc1(strlen(str)); // BAD (no room for `\0` terminator)
strcpy(buffer1, str);
}
{
char *buffer2 = (char *)MyMalloc2(strlen(str)); // BAD (no room for `\0` terminator)
strcpy(buffer2, str);
}
}

13
cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/TaintedAllocationSize/TaintedAllocationSize.expected
View File

@@ -10,12 +10,10 @@ edges
| test.cpp:148:20:148:25 | call to getenv | test.cpp:152:11:152:28 | ... * ... |
| test.cpp:209:8:209:23 | ReturnValue | test.cpp:241:9:241:24 | call to get_tainted_size |
| test.cpp:211:14:211:19 | call to getenv | test.cpp:209:8:209:23 | ReturnValue |
| test.cpp:224:23:224:23 | s | test.cpp:225:21:225:21 | s |
| test.cpp:230:21:230:21 | s | test.cpp:231:21:231:21 | s |
| test.cpp:237:24:237:29 | call to getenv | test.cpp:239:9:239:18 | local_size |
| test.cpp:237:24:237:29 | call to getenv | test.cpp:245:11:245:20 | local_size |
| test.cpp:237:24:237:29 | call to getenv | test.cpp:247:10:247:19 | local_size |
| test.cpp:245:11:245:20 | local_size | test.cpp:224:23:224:23 | s |
| test.cpp:247:10:247:19 | local_size | test.cpp:230:21:230:21 | s |
| test.cpp:251:2:251:9 | (reference dereference) [post update] | test.cpp:289:17:289:20 | size [post update] |
| test.cpp:251:2:251:9 | (reference dereference) [post update] | test.cpp:305:18:305:21 | size [post update] |
@@ -25,6 +23,8 @@ edges
| test.cpp:259:20:259:25 | call to getenv | test.cpp:263:11:263:29 | ... * ... |
| test.cpp:289:17:289:20 | size [post update] | test.cpp:291:11:291:28 | ... * ... |
| test.cpp:305:18:305:21 | size [post update] | test.cpp:308:10:308:27 | ... * ... |
| test.cpp:353:18:353:23 | call to getenv | test.cpp:355:35:355:38 | size |
| test.cpp:353:18:353:23 | call to getenv | test.cpp:356:35:356:38 | size |
nodes
| test.cpp:39:27:39:30 | argv | semmle.label | argv |
| test.cpp:43:38:43:44 | tainted | semmle.label | tainted |
@@ -41,8 +41,6 @@ nodes
| test.cpp:152:11:152:28 | ... * ... | semmle.label | ... * ... |
| test.cpp:209:8:209:23 | ReturnValue | semmle.label | ReturnValue |
| test.cpp:211:14:211:19 | call to getenv | semmle.label | call to getenv |
| test.cpp:224:23:224:23 | s | semmle.label | s |
| test.cpp:225:21:225:21 | s | semmle.label | s |
| test.cpp:230:21:230:21 | s | semmle.label | s |
| test.cpp:231:21:231:21 | s | semmle.label | s |
| test.cpp:237:24:237:29 | call to getenv | semmle.label | call to getenv |
@@ -58,6 +56,9 @@ nodes
| test.cpp:291:11:291:28 | ... * ... | semmle.label | ... * ... |
| test.cpp:305:18:305:21 | size [post update] | semmle.label | size [post update] |
| test.cpp:308:10:308:27 | ... * ... | semmle.label | ... * ... |
| test.cpp:353:18:353:23 | call to getenv | semmle.label | call to getenv |
| test.cpp:355:35:355:38 | size | semmle.label | size |
| test.cpp:356:35:356:38 | size | semmle.label | size |
subpaths
#select
| test.cpp:43:31:43:36 | call to malloc | test.cpp:39:27:39:30 | argv | test.cpp:43:38:43:44 | tainted | This allocation size is derived from $@ and might overflow. | test.cpp:39:27:39:30 | argv | user input (a command-line argument) |
@@ -69,10 +70,12 @@ subpaths
| test.cpp:128:17:128:22 | call to malloc | test.cpp:124:18:124:23 | call to getenv | test.cpp:128:24:128:41 | ... * ... | This allocation size is derived from $@ and might overflow. | test.cpp:124:18:124:23 | call to getenv | user input (an environment variable) |
| test.cpp:135:3:135:8 | call to malloc | test.cpp:133:19:133:24 | call to getenv | test.cpp:135:10:135:27 | ... * ... | This allocation size is derived from $@ and might overflow. | test.cpp:133:19:133:24 | call to getenv | user input (an environment variable) |
| test.cpp:152:4:152:9 | call to malloc | test.cpp:148:20:148:25 | call to getenv | test.cpp:152:11:152:28 | ... * ... | This allocation size is derived from $@ and might overflow. | test.cpp:148:20:148:25 | call to getenv | user input (an environment variable) |
| test.cpp:225:14:225:19 | call to malloc | test.cpp:237:24:237:29 | call to getenv | test.cpp:225:21:225:21 | s | This allocation size is derived from $@ and might overflow. | test.cpp:237:24:237:29 | call to getenv | user input (an environment variable) |
| test.cpp:231:14:231:19 | call to malloc | test.cpp:237:24:237:29 | call to getenv | test.cpp:231:21:231:21 | s | This allocation size is derived from $@ and might overflow. | test.cpp:237:24:237:29 | call to getenv | user input (an environment variable) |
| test.cpp:239:2:239:7 | call to malloc | test.cpp:237:24:237:29 | call to getenv | test.cpp:239:9:239:18 | local_size | This allocation size is derived from $@ and might overflow. | test.cpp:237:24:237:29 | call to getenv | user input (an environment variable) |
| test.cpp:241:2:241:7 | call to malloc | test.cpp:211:14:211:19 | call to getenv | test.cpp:241:9:241:24 | call to get_tainted_size | This allocation size is derived from $@ and might overflow. | test.cpp:211:14:211:19 | call to getenv | user input (an environment variable) |
| test.cpp:245:2:245:9 | call to my_alloc | test.cpp:237:24:237:29 | call to getenv | test.cpp:245:11:245:20 | local_size | This allocation size is derived from $@ and might overflow. | test.cpp:237:24:237:29 | call to getenv | user input (an environment variable) |
| test.cpp:263:4:263:9 | call to malloc | test.cpp:259:20:259:25 | call to getenv | test.cpp:263:11:263:29 | ... * ... | This allocation size is derived from $@ and might overflow. | test.cpp:259:20:259:25 | call to getenv | user input (an environment variable) |
| test.cpp:291:4:291:9 | call to malloc | test.cpp:251:18:251:23 | call to getenv | test.cpp:291:11:291:28 | ... * ... | This allocation size is derived from $@ and might overflow. | test.cpp:251:18:251:23 | call to getenv | user input (an environment variable) |
| test.cpp:308:3:308:8 | call to malloc | test.cpp:251:18:251:23 | call to getenv | test.cpp:308:10:308:27 | ... * ... | This allocation size is derived from $@ and might overflow. | test.cpp:251:18:251:23 | call to getenv | user input (an environment variable) |
| test.cpp:355:25:355:33 | call to MyMalloc1 | test.cpp:353:18:353:23 | call to getenv | test.cpp:355:35:355:38 | size | This allocation size is derived from $@ and might overflow. | test.cpp:353:18:353:23 | call to getenv | user input (an environment variable) |
| test.cpp:356:25:356:33 | call to MyMalloc2 | test.cpp:353:18:353:23 | call to getenv | test.cpp:356:35:356:38 | size | This allocation size is derived from $@ and might overflow. | test.cpp:353:18:353:23 | call to getenv | user input (an environment variable) |

17
cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/TaintedAllocationSize/test.cpp
View File

@@ -222,7 +222,7 @@ size_t get_bounded_size()
}
void *my_alloc(size_t s) {
void *ptr = malloc(s); // [UNHELPFUL RESULT]
void *ptr = malloc(s);
return ptr;
}
@@ -242,7 +242,7 @@ void more_cases() {
malloc(get_bounded_size()); // GOOD
my_alloc(100); // GOOD
my_alloc(local_size); // BAD [NOT DETECTED IN CORRECT LOCATION]
my_alloc(local_size); // BAD
my_func(100); // GOOD
my_func(local_size); // GOOD
}
@@ -342,3 +342,16 @@ void equality_barrier() {
int* a = (int*)malloc(size1 * sizeof(int)); // GOOD
}
}
// --- custom allocators ---
void *MyMalloc1(size_t size) { return malloc(size); }
void *MyMalloc2(size_t size);
void customAllocatorTests()
{
int size = atoi(getenv("USER"));
char *chars1 = (char *)MyMalloc1(size); // BAD
char *chars2 = (char *)MyMalloc2(size); // BAD
}