From 9e75a4be34c04a8c3154c6ef4a2e9f51d645db94 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 2 Feb 2021 16:42:39 +0100 Subject: [PATCH 1/2] C++: Implement a model for _strnextc and its variants. --- cpp/ql/src/semmle/code/cpp/models/Models.qll | 1 + .../cpp/models/implementations/Strnextc.qll | 38 +++++++++++++++++++ .../dataflow/taint-tests/localTaint.expected | 8 ++++ .../dataflow/taint-tests/taint.cpp | 14 +++++++ 4 files changed, 61 insertions(+) create mode 100644 cpp/ql/src/semmle/code/cpp/models/implementations/Strnextc.qll diff --git a/cpp/ql/src/semmle/code/cpp/models/Models.qll b/cpp/ql/src/semmle/code/cpp/models/Models.qll index 82998e9ccbc..7ec66b3a2e9 100644 --- a/cpp/ql/src/semmle/code/cpp/models/Models.qll +++ b/cpp/ql/src/semmle/code/cpp/models/Models.qll @@ -18,6 +18,7 @@ private import implementations.Strftime private import implementations.Strtok private import implementations.Strset private import implementations.Strcrement +private import implementations.Strnextc private import implementations.StdContainer private import implementations.StdPair private import implementations.StdMap diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Strnextc.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Strnextc.qll new file mode 100644 index 00000000000..fc8ac17b5f6 --- /dev/null +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Strnextc.qll @@ -0,0 +1,38 @@ +/** + * Provides implementation classes modeling `strnextc` and various similar functions. + * See `semmle.code.cpp.models.Models` for usage information. + */ + +import semmle.code.cpp.models.interfaces.ArrayFunction +import semmle.code.cpp.models.interfaces.Taint +import semmle.code.cpp.models.interfaces.Alias +import semmle.code.cpp.models.interfaces.SideEffect + +/** + * The function `strnextc` and its variants. + */ +private class Strnextc extends TaintFunction, ArrayFunction, AliasFunction, SideEffectFunction { + Strnextc() { this.hasGlobalName(["_strnextc", "_wcsnextc", "_mbsnextc", "_mbsnextc_l"]) } + + override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { + input.isParameterDeref(0) and output.isReturnValue() + } + + override predicate hasArrayWithNullTerminator(int bufParam) { bufParam = 0 } + + override predicate hasArrayInput(int bufParam) { bufParam = 0 } + + override predicate parameterNeverEscapes(int index) { index = 0 } + + override predicate parameterEscapesOnlyViaReturn(int index) { none() } + + override predicate parameterIsAlwaysReturned(int index) { none() } + + override predicate hasOnlySpecificReadSideEffects() { any() } + + override predicate hasOnlySpecificWriteSideEffects() { any() } + + override predicate hasSpecificReadSideEffect(ParameterIndex i, boolean buffer) { + i = 0 and buffer = true + } +} 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 b0eabbf5ca9..034bcdee517 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected @@ -6032,6 +6032,14 @@ | taint.cpp:616:26:616:30 | clean | taint.cpp:616:10:616:16 | call to _strdec | TAINT | | taint.cpp:617:7:617:11 | ref arg dest3 | taint.cpp:618:8:618:12 | dest3 | | | taint.cpp:618:8:618:12 | dest3 | taint.cpp:618:7:618:12 | * ... | TAINT | +| taint.cpp:625:33:625:38 | source | taint.cpp:628:17:628:22 | source | | +| taint.cpp:628:7:628:15 | call to _strnextc | taint.cpp:628:3:628:23 | ... = ... | | +| taint.cpp:628:7:628:15 | call to _strnextc | taint.cpp:629:8:629:8 | c | | +| taint.cpp:628:7:628:15 | call to _strnextc | taint.cpp:630:10:630:10 | c | | +| taint.cpp:628:17:628:22 | source | taint.cpp:628:7:628:15 | call to _strnextc | TAINT | +| taint.cpp:631:6:631:14 | call to _strnextc | taint.cpp:631:2:631:18 | ... = ... | | +| taint.cpp:631:6:631:14 | call to _strnextc | taint.cpp:632:7:632:7 | c | | +| taint.cpp:631:16:631:17 | | taint.cpp:631:6:631:14 | call to _strnextc | 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 91024f543bd..c51dc43706e 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp @@ -616,4 +616,18 @@ void test__strdec(const unsigned char* source, unsigned char* clean, unsigned ch dest3 = _strdec(source, clean); sink(dest3); // $ ast,ir sink(*dest3); // $ ast,ir +} + +// --- strnextc --- + +unsigned int _strnextc(const char*); + +void test__strnextc(const char* source) { + unsigned c = 0; + do { + c = _strnextc(source); + sink(c); // $ ast,ir + } while(c != '\0'); + c = _strnextc(""); + sink(c); } \ No newline at end of file From ff58d5a7c0210ebd52fe662784a78fd47fc9c862 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 2 Feb 2021 17:06:38 +0100 Subject: [PATCH 2/2] C++: Address review comments. --- .../library-tests/dataflow/taint-tests/localTaint.expected | 6 ++++-- cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) 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 034bcdee517..9f038101d67 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected @@ -6033,10 +6033,12 @@ | taint.cpp:617:7:617:11 | ref arg dest3 | taint.cpp:618:8:618:12 | dest3 | | | taint.cpp:618:8:618:12 | dest3 | taint.cpp:618:7:618:12 | * ... | TAINT | | taint.cpp:625:33:625:38 | source | taint.cpp:628:17:628:22 | source | | -| taint.cpp:628:7:628:15 | call to _strnextc | taint.cpp:628:3:628:23 | ... = ... | | +| taint.cpp:628:7:628:15 | call to _strnextc | taint.cpp:628:3:628:25 | ... = ... | | | taint.cpp:628:7:628:15 | call to _strnextc | taint.cpp:629:8:629:8 | c | | | taint.cpp:628:7:628:15 | call to _strnextc | taint.cpp:630:10:630:10 | c | | -| taint.cpp:628:17:628:22 | source | taint.cpp:628:7:628:15 | call to _strnextc | TAINT | +| taint.cpp:628:17:628:22 | source | taint.cpp:628:17:628:24 | ... ++ | | +| taint.cpp:628:17:628:24 | ... ++ | taint.cpp:628:7:628:15 | call to _strnextc | TAINT | +| taint.cpp:628:17:628:24 | ... ++ | taint.cpp:628:17:628:22 | source | TAINT | | taint.cpp:631:6:631:14 | call to _strnextc | taint.cpp:631:2:631:18 | ... = ... | | | taint.cpp:631:6:631:14 | call to _strnextc | taint.cpp:632:7:632:7 | c | | | taint.cpp:631:16:631:17 | | taint.cpp:631:6:631:14 | call to _strnextc | TAINT | 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 c51dc43706e..44d84c0c0b0 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp @@ -625,7 +625,7 @@ unsigned int _strnextc(const char*); void test__strnextc(const char* source) { unsigned c = 0; do { - c = _strnextc(source); + c = _strnextc(source++); sink(c); // $ ast,ir } while(c != '\0'); c = _strnextc("");