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

Merge pull request #14637 from MathiasVP/dataflow-for-realloc

Browse Source 
C++: Add a taint model for `realloc`
This commit is contained in:
Mathias Vorreiter Pedersen
2023-10-31 18:24:04 +01:00
committed by GitHub
parent ceb3d14fe0 377da9fa66
commit b79a5fee14
9 changed files with 55 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
cpp/ql/lib/change-notes/2023-10-30-realloc-flow.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Added taint models for `realloc` and related functions.

7
cpp/ql/lib/semmle/code/cpp/models/implementations/Allocation.qll
View File

@@ -5,6 +5,7 @@
*/
import semmle.code.cpp.models.interfaces.Allocation
import semmle.code.cpp.models.interfaces.Taint
/**
* An allocation function (such as `malloc`) that has an argument for the size
@@ -121,7 +122,7 @@ private class CallocAllocationFunction extends AllocationFunction {
* An allocation function (such as `realloc`) that has an argument for the size
* in bytes, and an argument for an existing pointer that is to be reallocated.
*/
private class ReallocAllocationFunction extends AllocationFunction {
private class ReallocAllocationFunction extends AllocationFunction, TaintFunction {
int sizeArg;
int reallocArg;
@@ -151,6 +152,10 @@ private class ReallocAllocationFunction extends AllocationFunction {
override int getSizeArg() { result = sizeArg }
override int getReallocPtrArg() { result = reallocArg }
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
input.isParameterDeref(this.getReallocPtrArg()) and output.isReturnValueDeref()
}
}
/**

16
cpp/ql/lib/semmle/code/cpp/models/implementations/Memset.qll
View File

@@ -9,18 +9,17 @@ import semmle.code.cpp.models.interfaces.DataFlow
import semmle.code.cpp.models.interfaces.Alias
import semmle.code.cpp.models.interfaces.SideEffect
/**
* The standard function `memset` and its assorted variants
*/
private class MemsetFunction extends ArrayFunction, DataFlowFunction, AliasFunction,
private class MemsetFunctionModel extends ArrayFunction, DataFlowFunction, AliasFunction,
SideEffectFunction
{
MemsetFunction() {
MemsetFunctionModel() {
this.hasGlobalOrStdOrBslName("memset")
or
this.hasGlobalOrStdName("wmemset")
or
this.hasGlobalName([bzero(), "__builtin_memset", "__builtin_memset_chk"])
this.hasGlobalName([
bzero(), "__builtin_memset", "__builtin_memset_chk", "RtlZeroMemory", "RtlSecureZeroMemory"
])
}
override predicate hasArrayOutput(int bufParam) { bufParam = 0 }
@@ -60,3 +59,8 @@ private class MemsetFunction extends ArrayFunction, DataFlowFunction, AliasFunct
}
private string bzero() { result = ["bzero", "explicit_bzero"] }
/**
* The standard function `memset` and its assorted variants
*/
class MemsetFunction extends Function instanceof MemsetFunctionModel { }

5
cpp/ql/src/Security/CWE/CWE-497/ExposedSystemData.ql
View File

@@ -15,6 +15,7 @@
import cpp
import semmle.code.cpp.ir.dataflow.TaintTracking
import semmle.code.cpp.models.interfaces.FlowSource
import semmle.code.cpp.models.implementations.Memset
import ExposedSystemData::PathGraph
import SystemData
@@ -28,6 +29,10 @@ module ExposedSystemDataConfig implements DataFlow::ConfigSig {
fc.getArgument(arg).getAChild*() = sink.asIndirectExpr()
)
}
predicate isBarrier(DataFlow::Node node) {
node.asIndirectArgument() = any(MemsetFunction func).getACallToThisFunction().getAnArgument()
}
}
module ExposedSystemData = TaintTracking::Global<ExposedSystemDataConfig>;

5
cpp/ql/src/Security/CWE/CWE-497/PotentiallyExposedSystemData.ql
View File

@@ -28,6 +28,7 @@ import cpp
import semmle.code.cpp.ir.dataflow.TaintTracking
import semmle.code.cpp.models.interfaces.FlowSource
import semmle.code.cpp.security.OutputWrite
import semmle.code.cpp.models.implementations.Memset
import PotentiallyExposedSystemData::PathGraph
import SystemData
@@ -49,6 +50,10 @@ module PotentiallyExposedSystemDataConfig implements DataFlow::ConfigSig {
else child = sink.asExpr()
)
}
predicate isBarrier(DataFlow::Node node) {
node.asIndirectArgument() = any(MemsetFunction func).getACallToThisFunction().getAnArgument()
}
}
module PotentiallyExposedSystemData = TaintTracking::Global<PotentiallyExposedSystemDataConfig>;

3
cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
View File

@@ -6643,6 +6643,9 @@ WARNING: Module TaintTracking has been deprecated and may be removed in future (
| taint.cpp:729:27:729:32 | endptr | taint.cpp:729:26:729:32 | & ... | |
| taint.cpp:731:7:731:12 | ref arg endptr | taint.cpp:732:8:732:13 | endptr | |
| taint.cpp:732:8:732:13 | endptr | taint.cpp:732:7:732:13 | * ... | TAINT |
| taint.cpp:738:17:738:31 | call to indirect_source | taint.cpp:739:30:739:35 | source | |
| taint.cpp:739:22:739:28 | call to realloc | taint.cpp:740:7:740:10 | dest | |
| taint.cpp:739:30:739:35 | source | taint.cpp:739:22:739:28 | call to realloc | TAINT |
| vector.cpp:16:43:16:49 | source1 | vector.cpp:17:26:17:32 | source1 | |
| vector.cpp:16:43:16:49 | source1 | vector.cpp:31:38:31:44 | source1 | |
| vector.cpp:17:21:17:33 | call to vector | vector.cpp:19:14:19:14 | v | |

8
cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp
View File

@@ -730,4 +730,12 @@ void test_strtol(char *source) {
sink(l); // $ ast,ir
sink(endptr); // $ ast,ir
sink(*endptr); // $ ast,ir
}
void *realloc(void *, size_t);
void test_realloc() {
char *source = indirect_source();
char *dest = (char*)realloc(source, 16);
sink(dest); // $ ir MISSING: ast
}

4
cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/PotentiallyExposedSystemData.expected
View File

@@ -10,6 +10,7 @@ edges
| tests.cpp:131:14:131:35 | call to getenv indirection | tests.cpp:107:30:107:32 | msg indirection |
| tests.cpp:132:14:132:35 | call to getenv indirection | tests.cpp:114:30:114:32 | msg indirection |
| tests.cpp:133:14:133:35 | call to getenv indirection | tests.cpp:122:30:122:32 | msg indirection |
| tests.cpp:139:17:139:22 | call to getenv indirection | tests.cpp:141:15:141:20 | secret indirection |
| tests_passwd.cpp:16:8:16:15 | call to getpwnam indirection | tests_passwd.cpp:18:29:18:31 | pwd indirection |
| tests_passwd.cpp:16:8:16:15 | call to getpwnam indirection | tests_passwd.cpp:19:26:19:28 | pwd indirection |
nodes
@@ -37,6 +38,8 @@ nodes
| tests.cpp:132:14:132:35 | call to getenv indirection | semmle.label | call to getenv indirection |
| tests.cpp:133:14:133:35 | call to getenv indirection | semmle.label | call to getenv indirection |
| tests.cpp:133:14:133:35 | call to getenv indirection | semmle.label | call to getenv indirection |
| tests.cpp:139:17:139:22 | call to getenv indirection | semmle.label | call to getenv indirection |
| tests.cpp:141:15:141:20 | secret indirection | semmle.label | secret indirection |
| tests_passwd.cpp:16:8:16:15 | call to getpwnam indirection | semmle.label | call to getpwnam indirection |
| tests_passwd.cpp:18:29:18:31 | pwd indirection | semmle.label | pwd indirection |
| tests_passwd.cpp:19:26:19:28 | pwd indirection | semmle.label | pwd indirection |
@@ -56,5 +59,6 @@ subpaths
| tests.cpp:119:7:119:12 | buffer indirection | tests.cpp:132:14:132:35 | call to getenv indirection | tests.cpp:119:7:119:12 | buffer indirection | This operation potentially exposes sensitive system data from $@. | tests.cpp:132:14:132:35 | call to getenv indirection | call to getenv indirection |
| tests.cpp:124:15:124:17 | msg indirection | tests.cpp:133:14:133:35 | call to getenv indirection | tests.cpp:124:15:124:17 | msg indirection | This operation potentially exposes sensitive system data from $@. | tests.cpp:133:14:133:35 | call to getenv indirection | call to getenv indirection |
| tests.cpp:133:14:133:35 | call to getenv indirection | tests.cpp:133:14:133:35 | call to getenv indirection | tests.cpp:133:14:133:35 | call to getenv indirection | This operation potentially exposes sensitive system data from $@. | tests.cpp:133:14:133:35 | call to getenv indirection | call to getenv indirection |
| tests.cpp:141:15:141:20 | secret indirection | tests.cpp:139:17:139:22 | call to getenv indirection | tests.cpp:141:15:141:20 | secret indirection | This operation potentially exposes sensitive system data from $@. | tests.cpp:139:17:139:22 | call to getenv indirection | call to getenv indirection |
| tests_passwd.cpp:18:29:18:31 | pwd indirection | tests_passwd.cpp:16:8:16:15 | call to getpwnam indirection | tests_passwd.cpp:18:29:18:31 | pwd indirection | This operation potentially exposes sensitive system data from $@. | tests_passwd.cpp:16:8:16:15 | call to getpwnam indirection | call to getpwnam indirection |
| tests_passwd.cpp:19:26:19:28 | pwd indirection | tests_passwd.cpp:16:8:16:15 | call to getpwnam indirection | tests_passwd.cpp:19:26:19:28 | pwd indirection | This operation potentially exposes sensitive system data from $@. | tests_passwd.cpp:16:8:16:15 | call to getpwnam indirection | call to getpwnam indirection |

10
cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/tests.cpp
View File

@@ -132,3 +132,13 @@ void test5()
myOutputFn4(getenv("SECRET_TOKEN")); // BAD: outputs the SECRET_TOKEN environment variable
myOutputFn5(getenv("SECRET_TOKEN")); // BAD: outputs the SECRET_TOKEN environment variable
}
void RtlZeroMemory(void* dst, size_t len);
void test_clear_memory(char *username) {
char* secret = getenv("SECRET_TOKEN");
printf("%s", secret); // BAD
RtlZeroMemory(secret, 1024);
printf("%s", secret); // GOOD
}