diff --git a/cpp/ql/lib/change-notes/2023-10-30-realloc-flow.md b/cpp/ql/lib/change-notes/2023-10-30-realloc-flow.md new file mode 100644 index 000000000000..69f00c5a8206 --- /dev/null +++ b/cpp/ql/lib/change-notes/2023-10-30-realloc-flow.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Added taint models for `realloc` and related functions. diff --git a/cpp/ql/lib/semmle/code/cpp/models/implementations/Allocation.qll b/cpp/ql/lib/semmle/code/cpp/models/implementations/Allocation.qll index 5aa421b2bcf3..305a0c257322 100644 --- a/cpp/ql/lib/semmle/code/cpp/models/implementations/Allocation.qll +++ b/cpp/ql/lib/semmle/code/cpp/models/implementations/Allocation.qll @@ -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() + } } /** diff --git a/cpp/ql/lib/semmle/code/cpp/models/implementations/Memset.qll b/cpp/ql/lib/semmle/code/cpp/models/implementations/Memset.qll index 0d09173854ca..a540c0a88b60 100644 --- a/cpp/ql/lib/semmle/code/cpp/models/implementations/Memset.qll +++ b/cpp/ql/lib/semmle/code/cpp/models/implementations/Memset.qll @@ -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 { } diff --git a/cpp/ql/src/Security/CWE/CWE-497/ExposedSystemData.ql b/cpp/ql/src/Security/CWE/CWE-497/ExposedSystemData.ql index dcbaedea42c8..e8aee0e49ca1 100644 --- a/cpp/ql/src/Security/CWE/CWE-497/ExposedSystemData.ql +++ b/cpp/ql/src/Security/CWE/CWE-497/ExposedSystemData.ql @@ -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; diff --git a/cpp/ql/src/Security/CWE/CWE-497/PotentiallyExposedSystemData.ql b/cpp/ql/src/Security/CWE/CWE-497/PotentiallyExposedSystemData.ql index 9fa4f538378b..f3c9ca189b94 100644 --- a/cpp/ql/src/Security/CWE/CWE-497/PotentiallyExposedSystemData.ql +++ b/cpp/ql/src/Security/CWE/CWE-497/PotentiallyExposedSystemData.ql @@ -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; diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected index ceb3cde9a8e3..f48d71cd1ab6 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected @@ -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 | | diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp index e479d7a11e01..da2c8034c4b1 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp @@ -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 } \ No newline at end of file diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/PotentiallyExposedSystemData.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/PotentiallyExposedSystemData.expected index 840cd6ed5f33..78d76bf7411d 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/PotentiallyExposedSystemData.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/PotentiallyExposedSystemData.expected @@ -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 | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/tests.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/tests.cpp index 843d579386b9..25a071bee467 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/tests.cpp +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/tests.cpp @@ -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 +} \ No newline at end of file