From ff788c93c0dfc7969990ca6659bef4b74327dbf8 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 30 Jul 2024 10:13:39 +0100 Subject: [PATCH 1/9] C++: Add a model for 'std::format' and a failing test. --- .../dataflow/taint-tests/format.cpp | 7 +++- .../dataflow/taint-tests/localTaint.expected | 2 ++ .../library-tests/dataflow/taint-tests/stl.h | 35 +++++++++++++++++++ .../taint-tests/test_mad-signatures.expected | 1 + 4 files changed, 44 insertions(+), 1 deletion(-) diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/format.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/format.cpp index c6b877e9ca83..9cf16d7d5889 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/format.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/format.cpp @@ -1,4 +1,4 @@ - +#include "stl.h" typedef unsigned long size_t; typedef struct {} FILE; @@ -157,3 +157,8 @@ void test2() sink(s[strlen(s) - 1]); // $ ast,ir sink(ws + (wcslen(ws) / 2)); // $ ast,ir } + +void test_format() { + auto s = std::format("{}", string::source()); + sink(s); // $ MISSING: ast,ir +} \ No newline at end of file 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 1e8087c7ec96..d565f6d80e08 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected @@ -447,6 +447,8 @@ WARNING: module 'TaintTracking' has been deprecated and may be removed in future | format.cpp:158:13:158:18 | call to wcslen | format.cpp:158:13:158:26 | ... / ... | TAINT | | format.cpp:158:13:158:26 | ... / ... | format.cpp:158:7:158:27 | ... + ... | TAINT | | format.cpp:158:26:158:26 | 2 | format.cpp:158:13:158:26 | ... / ... | TAINT | +| format.cpp:162:12:162:22 | call to format | format.cpp:163:8:163:8 | s | | +| format.cpp:162:24:162:27 | {} | format.cpp:162:24:162:27 | call to basic_format_string | TAINT | | map.cpp:21:28:21:28 | call to pair | map.cpp:23:2:23:2 | a | | | map.cpp:21:28:21:28 | call to pair | map.cpp:24:7:24:7 | a | | | map.cpp:21:28:21:28 | call to pair | map.cpp:25:7:25:7 | a | | diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/stl.h b/cpp/ql/test/library-tests/dataflow/taint-tests/stl.h index 44550c3df768..9647f41cb7c4 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/stl.h +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/stl.h @@ -642,3 +642,38 @@ namespace std { pair equal_range(const key_type& k) const; }; } + +// --- string view --- + +namespace std { + template> + class basic_string_view { + public: + using size_type = size_t; + + basic_string_view() noexcept; + basic_string_view(const basic_string_view&) noexcept; + basic_string_view(const CharT*, size_type); + basic_string_view(const CharT*); + template basic_string_view(It, End); + template explicit basic_string_view(R&&); + basic_string_view& operator=(const basic_string_view&) noexcept; + }; + + using string_view = basic_string_view; +} + +// --- format --- +namespace std { + template + struct basic_format_string { + public: + template basic_format_string(const T&); + + basic_string_view get() const noexcept; + }; + + using format_string = basic_format_string; // simplified from `char, std::type_identity_t...` + + template string format( format_string fmt, Args&&... args ); +} \ No newline at end of file diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.expected index 45436051f79c..79425ce2d97d 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.expected @@ -650,3 +650,4 @@ getParameterTypeName | stl.h:636:37:636:41 | merge | 0 | unordered_set & | | stl.h:639:12:639:15 | find | 0 | const key_type & | | stl.h:641:28:641:38 | equal_range | 0 | const key_type & | +| stl.h:671:21:671:39 | basic_format_string | 0 | const func:0 & | From 8edf19adc0dbca84dba7decb31987652a6c91927 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 30 Jul 2024 10:14:41 +0100 Subject: [PATCH 2/9] C++: Add MaD model for 'std::format'. --- cpp/ql/lib/ext/std.format.model.yml | 13 ++++++++++++ .../taint-tests/test_mad-signatures.expected | 20 +++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 cpp/ql/lib/ext/std.format.model.yml diff --git a/cpp/ql/lib/ext/std.format.model.yml b/cpp/ql/lib/ext/std.format.model.yml new file mode 100644 index 000000000000..dbd547009556 --- /dev/null +++ b/cpp/ql/lib/ext/std.format.model.yml @@ -0,0 +1,13 @@ +extensions: + - addsTo: + pack: codeql/cpp-all + extensible: summaryModel + data: # namespace, type, subtypes, name, signature, ext, input, output, kind, provenance + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*1]", "ReturnValue.Element[@]", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*2]", "ReturnValue.Element[@]", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*3]", "ReturnValue.Element[@]", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*4]", "ReturnValue.Element[@]", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*5]", "ReturnValue.Element[@]", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*6]", "ReturnValue.Element[@]", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*7]", "ReturnValue.Element[@]", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*8]", "ReturnValue.Element[@]", "taint", "manual"] \ No newline at end of file diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.expected index 79425ce2d97d..d61cbaa1f2ce 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.expected @@ -175,6 +175,7 @@ signatureMatches | stl.h:333:42:333:47 | insert | (const_iterator,InputIt,InputIt) | vector | insert | 1 | | stl.h:333:42:333:47 | insert | (const_iterator,InputIt,InputIt) | vector | insert | 2 | | stl.h:333:42:333:47 | insert | (const_iterator,InputIt,InputIt) | vector | insert | 2 | +| stl.h:335:37:335:43 | emplace | (format_string,Args &&) | | format | 1 | | stl.h:396:3:396:3 | pair | (const deque &,const Allocator &) | deque | deque | 1 | | stl.h:396:3:396:3 | pair | (const deque &,const Allocator &) | deque | deque | 1 | | stl.h:396:3:396:3 | pair | (const deque &,const Allocator &) | deque | deque | 1 | @@ -215,6 +216,19 @@ signatureMatches | stl.h:396:3:396:3 | pair | (vector &&,const Allocator &) | vector | vector | 1 | | stl.h:396:3:396:3 | pair | (vector &&,const Allocator &) | vector | vector | 1 | | stl.h:396:3:396:3 | pair | (vector &&,const Allocator &) | vector | vector | 1 | +| stl.h:440:36:440:47 | emplace_hint | (format_string,Args &&) | | format | 1 | +| stl.h:440:36:440:47 | emplace_hint | (format_string,Args &&) | | format | 1 | +| stl.h:448:48:448:58 | try_emplace | (format_string,Args &&) | | format | 1 | +| stl.h:448:48:448:58 | try_emplace | (format_string,Args &&) | | format | 1 | +| stl.h:452:42:452:57 | insert_or_assign | (format_string,Args &&) | | format | 1 | +| stl.h:508:36:508:47 | emplace_hint | (format_string,Args &&) | | format | 1 | +| stl.h:508:36:508:47 | emplace_hint | (format_string,Args &&) | | format | 1 | +| stl.h:516:48:516:58 | try_emplace | (format_string,Args &&) | | format | 1 | +| stl.h:516:48:516:58 | try_emplace | (format_string,Args &&) | | format | 1 | +| stl.h:516:48:516:58 | try_emplace | (format_string,Args &&) | | format | 1 | +| stl.h:516:48:516:58 | try_emplace | (format_string,Args &&) | | format | 1 | +| stl.h:516:48:516:58 | try_emplace | (format_string,Args &&) | | format | 1 | +| stl.h:520:42:520:57 | insert_or_assign | (format_string,Args &&) | | format | 1 | | stl.h:557:33:557:35 | set | (InputIt,InputIt) | deque | assign | 0 | | stl.h:557:33:557:35 | set | (InputIt,InputIt) | deque | assign | 1 | | stl.h:557:33:557:35 | set | (InputIt,InputIt) | forward_list | assign | 0 | @@ -223,6 +237,8 @@ signatureMatches | stl.h:557:33:557:35 | set | (InputIt,InputIt) | list | assign | 1 | | stl.h:557:33:557:35 | set | (InputIt,InputIt) | vector | assign | 0 | | stl.h:557:33:557:35 | set | (InputIt,InputIt) | vector | assign | 1 | +| stl.h:569:36:569:47 | emplace_hint | (format_string,Args &&) | | format | 1 | +| stl.h:569:36:569:47 | emplace_hint | (format_string,Args &&) | | format | 1 | | stl.h:574:38:574:43 | insert | (InputIt,InputIt) | deque | assign | 0 | | stl.h:574:38:574:43 | insert | (InputIt,InputIt) | deque | assign | 1 | | stl.h:574:38:574:43 | insert | (InputIt,InputIt) | forward_list | assign | 0 | @@ -231,6 +247,8 @@ signatureMatches | stl.h:574:38:574:43 | insert | (InputIt,InputIt) | list | assign | 1 | | stl.h:574:38:574:43 | insert | (InputIt,InputIt) | vector | assign | 0 | | stl.h:574:38:574:43 | insert | (InputIt,InputIt) | vector | assign | 1 | +| stl.h:623:36:623:47 | emplace_hint | (format_string,Args &&) | | format | 1 | +| stl.h:623:36:623:47 | emplace_hint | (format_string,Args &&) | | format | 1 | | stl.h:628:38:628:43 | insert | (InputIt,InputIt) | deque | assign | 0 | | stl.h:628:38:628:43 | insert | (InputIt,InputIt) | deque | assign | 1 | | stl.h:628:38:628:43 | insert | (InputIt,InputIt) | forward_list | assign | 0 | @@ -315,6 +333,8 @@ getSignatureParameterName | (deque &&) | deque | deque | 0 | deque && | | (deque &&,const Allocator &) | deque | deque | 0 | deque && | | (deque &&,const Allocator &) | deque | deque | 1 | const class:1 & | +| (format_string,Args &&) | | format | 0 | format_string | +| (format_string,Args &&) | | format | 1 | func:0 && | | (forward_list &&) | forward_list | forward_list | 0 | forward_list && | | (forward_list &&,const Allocator &) | forward_list | forward_list | 0 | forward_list && | | (forward_list &&,const Allocator &) | forward_list | forward_list | 1 | const class:1 & | From 6b37cb0718baab1a7a5734e8602ade2f64b65935 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 30 Jul 2024 10:17:34 +0100 Subject: [PATCH 3/9] C++: Use the same 'template expansion mechanism' for free functions that we use for member functions. --- .../semmle/code/cpp/dataflow/ExternalFlow.qll | 61 +++++++++++++------ 1 file changed, 41 insertions(+), 20 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll index 73456e457170..2a92682fc702 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll @@ -435,12 +435,17 @@ private predicate elementSpec( } /** Gets the fully templated version of `f`. */ -private Function getFullyTemplatedMemberFunction(Function f) { +private Function getFullyTemplatedFunction(Function f) { not f.isFromUninstantiatedTemplate(_) and - exists(Class c, Class templateClass, int i | - c.isConstructedFrom(templateClass) and - f = c.getAMember(i) and - result = templateClass.getCanonicalMember(i) + ( + exists(Class c, Class templateClass, int i | + c.isConstructedFrom(templateClass) and + f = c.getAMember(i) and + result = templateClass.getCanonicalMember(i) + ) + or + not exists(f.getDeclaringType()) and + f.isConstructedFrom(result) ) } @@ -464,14 +469,14 @@ string getParameterTypeWithoutTemplateArguments(Function f, int n) { */ private string getTypeNameWithoutFunctionTemplates(Function f, int n, int remaining) { exists(Function templateFunction | - templateFunction = getFullyTemplatedMemberFunction(f) and + templateFunction = getFullyTemplatedFunction(f) and remaining = templateFunction.getNumberOfTemplateArguments() and result = getParameterTypeWithoutTemplateArguments(templateFunction, n) ) or exists(string mid, TemplateParameter tp, Function templateFunction | mid = getTypeNameWithoutFunctionTemplates(f, n, remaining + 1) and - templateFunction = getFullyTemplatedMemberFunction(f) and + templateFunction = getFullyTemplatedFunction(f) and tp = templateFunction.getTemplateArgument(remaining) and result = mid.replaceAll(tp.getName(), "func:" + remaining.toString()) ) @@ -482,12 +487,18 @@ private string getTypeNameWithoutFunctionTemplates(Function f, int n, int remain * with `class:N` (where `N` is the index of the template). */ private string getTypeNameWithoutClassTemplates(Function f, int n, int remaining) { + // If there is a declaring type then we start by expanding the function templates exists(Class template | f.getDeclaringType().isConstructedFrom(template) and remaining = template.getNumberOfTemplateArguments() and result = getTypeNameWithoutFunctionTemplates(f, n, 0) ) or + // If there is no declaring type we're done after expanding the function templates + not exists(f.getDeclaringType()) and + remaining = 0 and + result = getTypeNameWithoutFunctionTemplates(f, n, 0) + or exists(string mid, TemplateParameter tp, Class template | mid = getTypeNameWithoutClassTemplates(f, n, remaining + 1) and f.getDeclaringType().isConstructedFrom(template) and @@ -750,17 +761,17 @@ private predicate elementSpecWithArguments0( /** * Holds if `elementSpec(namespace, type, subtypes, name, signature, _)` and - * `method`'s signature matches `signature`. + * `func`'s signature matches `signature`. * * `signature` may contain template parameter names that are bound by `type` and `name`. */ pragma[nomagic] private predicate elementSpecMatchesSignature( - Function method, string namespace, string type, boolean subtypes, string name, string signature + Function func, string namespace, string type, boolean subtypes, string name, string signature ) { elementSpec(namespace, pragma[only_bind_into](type), subtypes, pragma[only_bind_into](name), pragma[only_bind_into](signature), _) and - signatureMatches(method, signature, type, name, 0) + signatureMatches(func, signature, type, name, 0) } /** @@ -776,13 +787,22 @@ private predicate hasClassAndName(Class classWithMethod, Function method, string ) } +bindingset[name] +pragma[inline_late] +private predicate funcHasQualifiedName(Function func, string namespace, string name) { + exists(string nameWithoutArgs | + parseAngles(name, nameWithoutArgs, _, "") and + func.hasQualifiedName(namespace, name) + ) +} + /** * Holds if `namedClass` is in namespace `namespace` and has * name `type` (excluding any template parameters). */ bindingset[type, namespace] pragma[inline_late] -private predicate hasQualifiedName(Class namedClass, string namespace, string type) { +private predicate classHasQualifiedName(Class namedClass, string namespace, string type) { exists(string typeWithoutArgs | parseAngles(type, typeWithoutArgs, _, "") and namedClass.hasQualifiedName(namespace, typeWithoutArgs) @@ -804,15 +824,16 @@ private Element interpretElement0( string namespace, string type, boolean subtypes, string name, string signature ) { ( - elementSpec(namespace, type, subtypes, name, signature, _) and // Non-member functions - exists(Function func | - func.hasQualifiedName(namespace, name) and - type = "" and - matchesSignature(func, signature) and - subtypes = false and - not exists(func.getDeclaringType()) and - result = func + elementSpec(namespace, type, subtypes, name, signature, _) and + subtypes = false and + type = "" and + ( + elementSpecMatchesSignature(result, namespace, type, subtypes, name, signature) + or + signature = "" and + elementSpec(namespace, type, subtypes, name, "", _) and + funcHasQualifiedName(result, namespace, name) ) or // Member functions @@ -825,7 +846,7 @@ private Element interpretElement0( elementSpec(namespace, type, subtypes, name, "", _) and hasClassAndName(classWithMethod, result, name) ) and - hasQualifiedName(namedClass, namespace, type) and + classHasQualifiedName(namedClass, namespace, type) and ( // member declared in the named type or a subtype of it subtypes = true and From 68a972d5789655b451d42c1f4d8690201b102432 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 30 Jul 2024 10:18:04 +0100 Subject: [PATCH 4/9] C++: Delete code that's now dead. --- .../semmle/code/cpp/dataflow/ExternalFlow.qll | 32 ------------------- 1 file changed, 32 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll index 2a92682fc702..43e9f9d616f9 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll @@ -581,38 +581,6 @@ private string getSignatureWithoutFunctionTemplateNames( ) } -private string paramsStringPart(Function c, int i) { - not c.isFromUninstantiatedTemplate(_) and - ( - i = -1 and result = "(" and exists(c) - or - exists(int n, string p | getParameterTypeName(c, n) = p | - i = 2 * n and result = p - or - i = 2 * n - 1 and result = "," and n != 0 - ) - or - i = 2 * c.getNumberOfParameters() and result = ")" - ) -} - -/** - * Gets a parenthesized string containing all parameter types of this callable, separated by a comma. - * - * Returns the empty string if the callable has no parameters. - * Parameter types are represented by their type erasure. - */ -cached -private string paramsString(Function c) { - result = concat(int i | | paramsStringPart(c, i) order by i) -} - -bindingset[func] -private predicate matchesSignature(Function func, string signature) { - signature = "" or - paramsString(func) = signature -} - /** * Holds if `elementSpec(_, type, _, name, signature, _)` holds and * - `typeArgs` represents the named template parameters supplied to `type`, and From f932e515a739049279762ea8a71b21da7e44c6b5 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 30 Jul 2024 10:20:00 +0100 Subject: [PATCH 5/9] C++: Accept test changes. --- .../dataflow/taint-tests/format.cpp | 2 +- .../taint-tests/test_mad-signatures.expected | 62 +++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/format.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/format.cpp index 9cf16d7d5889..a5cfadea464b 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/format.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/format.cpp @@ -160,5 +160,5 @@ void test2() void test_format() { auto s = std::format("{}", string::source()); - sink(s); // $ MISSING: ast,ir + sink(s); // $ ir MISSING: ast } \ No newline at end of file diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.expected index d61cbaa1f2ce..209a02094fef 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.expected @@ -27,6 +27,10 @@ signatureMatches | stl.h:190:17:190:23 | replace | (const_iterator,InputIt,InputIt) | vector | insert | 0 | | stl.h:190:17:190:23 | replace | (const_iterator,InputIt,InputIt) | vector | insert | 1 | | stl.h:190:17:190:23 | replace | (const_iterator,InputIt,InputIt) | vector | insert | 2 | +| stl.h:232:84:232:90 | getline | (const_iterator,InputIt,InputIt) | deque | insert | 2 | +| stl.h:232:84:232:90 | getline | (const_iterator,InputIt,InputIt) | forward_list | insert_after | 2 | +| stl.h:232:84:232:90 | getline | (const_iterator,InputIt,InputIt) | list | insert | 2 | +| stl.h:232:84:232:90 | getline | (const_iterator,InputIt,InputIt) | vector | insert | 2 | | stl.h:294:12:294:17 | vector | (const deque &,const Allocator &) | deque | deque | 1 | | stl.h:294:12:294:17 | vector | (const deque &,const Allocator &) | deque | deque | 1 | | stl.h:294:12:294:17 | vector | (const deque &,const Allocator &) | deque | deque | 1 | @@ -257,6 +261,8 @@ signatureMatches | stl.h:628:38:628:43 | insert | (InputIt,InputIt) | list | assign | 1 | | stl.h:628:38:628:43 | insert | (InputIt,InputIt) | vector | assign | 0 | | stl.h:628:38:628:43 | insert | (InputIt,InputIt) | vector | assign | 1 | +| stl.h:678:33:678:38 | format | (format_string,Args &&) | | format | 0 | +| stl.h:678:33:678:38 | format | (format_string,Args &&) | | format | 1 | getSignatureParameterName | (InputIt,InputIt) | deque | assign | 0 | func:0 | | (InputIt,InputIt) | deque | assign | 1 | func:0 | @@ -365,6 +371,14 @@ getSignatureParameterName | (vector &&,const Allocator &) | vector | vector | 0 | vector && | | (vector &&,const Allocator &) | vector | vector | 1 | const class:1 & | getParameterTypeName +| smart_pointer.cpp:7:27:7:30 | sink | 0 | shared_ptr & | +| smart_pointer.cpp:7:27:7:30 | sink | 0 | shared_ptr & | +| smart_pointer.cpp:8:27:8:30 | sink | 0 | unique_ptr & | +| smart_pointer.cpp:8:27:8:30 | sink | 0 | unique_ptr & | +| stl.h:29:34:29:40 | forward | 0 | remove_reference_t & | +| stl.h:29:34:29:40 | forward | 0 | remove_reference_t & | +| stl.h:29:34:29:40 | forward | 0 | remove_reference_t & | +| stl.h:29:34:29:40 | forward | 0 | remove_reference_t & | | stl.h:49:3:49:10 | iterator | 0 | const iterator & | | stl.h:49:3:49:10 | iterator | 0 | const iterator & | | stl.h:49:3:49:10 | iterator | 0 | const iterator & | @@ -397,6 +411,8 @@ getParameterTypeName | stl.h:88:25:88:33 | operator= | 0 | value_type && | | stl.h:91:24:91:33 | operator++ | 0 | int | | stl.h:91:24:91:33 | operator++ | 0 | int | +| stl.h:95:44:95:44 | back_inserter | 0 | func:0 & | +| stl.h:95:44:95:44 | back_inserter | 0 | func:0 & | | stl.h:148:3:148:14 | basic_string | 0 | const class:2 & | | stl.h:149:33:149:44 | basic_string | 0 | const class:0 * | | stl.h:149:33:149:44 | basic_string | 1 | const class:2 & | @@ -445,6 +461,10 @@ getParameterTypeName | stl.h:193:8:193:12 | clear | 2 | size_type | | stl.h:195:8:195:11 | swap | 0 | size_type | | stl.h:195:8:195:11 | swap | 1 | size_type | +| stl.h:198:94:198:102 | operator+ | 0 | const basic_string & | +| stl.h:198:94:198:102 | operator+ | 1 | const basic_string & | +| stl.h:199:94:199:102 | operator+ | 0 | const basic_string & | +| stl.h:199:94:199:102 | operator+ | 1 | const func:0 * | | stl.h:214:33:214:42 | operator>> | 0 | int & | | stl.h:217:33:217:35 | get | 0 | char_type & | | stl.h:218:33:218:35 | get | 0 | char_type * | @@ -459,10 +479,23 @@ getParameterTypeName | stl.h:226:32:226:38 | getline | 0 | char_type * | | stl.h:226:32:226:38 | getline | 1 | streamsize | | stl.h:226:32:226:38 | getline | 2 | char_type | +| stl.h:229:68:229:77 | operator>> | 0 | basic_istream & | +| stl.h:229:68:229:77 | operator>> | 1 | func:0 * | +| stl.h:230:85:230:94 | operator>> | 0 | basic_istream & | +| stl.h:230:85:230:94 | operator>> | 1 | basic_string & | +| stl.h:232:84:232:90 | getline | 0 | basic_istream & | +| stl.h:232:84:232:90 | getline | 1 | basic_string & | +| stl.h:232:84:232:90 | getline | 2 | func:0 | +| stl.h:233:84:233:90 | getline | 0 | basic_istream & | +| stl.h:233:84:233:90 | getline | 1 | basic_string & | | stl.h:240:33:240:42 | operator<< | 0 | int | | stl.h:242:33:242:35 | put | 0 | char_type | | stl.h:243:33:243:37 | write | 0 | const char_type * | | stl.h:243:33:243:37 | write | 1 | streamsize | +| stl.h:247:67:247:76 | operator<< | 0 | basic_ostream & | +| stl.h:247:67:247:76 | operator<< | 1 | const func:0 * | +| stl.h:248:85:248:94 | operator<< | 0 | basic_ostream & | +| stl.h:248:85:248:94 | operator<< | 1 | const basic_string & | | stl.h:259:12:259:29 | basic_stringstream | 0 | const basic_string & | | stl.h:263:23:263:31 | operator= | 0 | basic_stringstream && | | stl.h:265:8:265:11 | swap | 0 | basic_stringstream & | @@ -524,6 +557,12 @@ getParameterTypeName | stl.h:352:3:352:12 | shared_ptr | 0 | const shared_ptr & | | stl.h:352:3:352:12 | shared_ptr | 0 | const shared_ptr & | | stl.h:369:12:369:21 | unique_ptr | 0 | class:0 * | +| stl.h:380:52:380:62 | make_unique | 0 | func:1 && | +| stl.h:380:52:380:62 | make_unique | 0 | func:1 && | +| stl.h:380:52:380:62 | make_unique | 0 | func:1 && | +| stl.h:382:52:382:62 | make_shared | 0 | func:1 && | +| stl.h:382:52:382:62 | make_shared | 0 | func:1 && | +| stl.h:382:52:382:62 | make_shared | 0 | func:1 && | | stl.h:396:3:396:3 | pair | 0 | const class:0 & | | stl.h:396:3:396:3 | pair | 0 | const class:0 & | | stl.h:396:3:396:3 | pair | 0 | const class:0 & | @@ -547,6 +586,18 @@ getParameterTypeName | stl.h:397:30:397:33 | pair | 0 | const pair & | | stl.h:397:30:397:33 | pair | 0 | const pair & | | stl.h:399:8:399:11 | swap | 0 | pair & | +| stl.h:402:72:402:72 | make_pair | 0 | func:0 && | +| stl.h:402:72:402:72 | make_pair | 0 | func:0 && | +| stl.h:402:72:402:72 | make_pair | 0 | func:0 && | +| stl.h:402:72:402:72 | make_pair | 0 | func:0 && | +| stl.h:402:72:402:72 | make_pair | 0 | func:0 && | +| stl.h:402:72:402:72 | make_pair | 0 | func:0 && | +| stl.h:402:72:402:72 | make_pair | 1 | func:1 && | +| stl.h:402:72:402:72 | make_pair | 1 | func:1 && | +| stl.h:402:72:402:72 | make_pair | 1 | func:1 && | +| stl.h:402:72:402:72 | make_pair | 1 | func:1 && | +| stl.h:402:72:402:72 | make_pair | 1 | func:1 && | +| stl.h:402:72:402:72 | make_pair | 1 | func:1 && | | stl.h:422:3:422:5 | map | 0 | const map & | | stl.h:426:8:426:16 | operator= | 0 | const map & | | stl.h:435:6:435:15 | operator[] | 0 | key_type && | @@ -671,3 +722,14 @@ getParameterTypeName | stl.h:639:12:639:15 | find | 0 | const key_type & | | stl.h:641:28:641:38 | equal_range | 0 | const key_type & | | stl.h:671:21:671:39 | basic_format_string | 0 | const func:0 & | +| stl.h:678:33:678:38 | format | 0 | format_string | +| stl.h:678:33:678:38 | format | 1 | func:0 && | +| stringstream.cpp:18:6:18:9 | sink | 0 | const basic_ostream> & | +| stringstream.cpp:21:6:21:9 | sink | 0 | const basic_istream> & | +| stringstream.cpp:24:6:24:9 | sink | 0 | const basic_iostream> & | +| swap1.cpp:14:9:14:9 | move | 0 | func:0 & | +| swap2.cpp:14:9:14:9 | move | 0 | func:0 & | +| swap.h:4:20:4:23 | swap | 0 | func:0 & | +| swap.h:4:20:4:23 | swap | 1 | func:0 & | +| vector.cpp:14:27:14:30 | sink | 0 | vector> & | +| vector.cpp:14:27:14:30 | sink | 0 | vector> & | From 42ee501b96aba8e6f582ae3a2a07a79775455cd4 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 30 Jul 2024 11:18:24 +0100 Subject: [PATCH 6/9] C++: Use the name without args --- cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll index 43e9f9d616f9..02300e41de15 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll @@ -760,7 +760,7 @@ pragma[inline_late] private predicate funcHasQualifiedName(Function func, string namespace, string name) { exists(string nameWithoutArgs | parseAngles(name, nameWithoutArgs, _, "") and - func.hasQualifiedName(namespace, name) + func.hasQualifiedName(namespace, nameWithoutArgs) ) } From afc0d0a078887080d31492facade18f6e86a0828 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 30 Jul 2024 11:54:20 +0100 Subject: [PATCH 7/9] C++: Accept test changes. --- .../dataflow/external-models/validatemodels.expected | 1 + 1 file changed, 1 insertion(+) diff --git a/cpp/ql/test/library-tests/dataflow/external-models/validatemodels.expected b/cpp/ql/test/library-tests/dataflow/external-models/validatemodels.expected index 4086b2c99568..2e0a493585c6 100644 --- a/cpp/ql/test/library-tests/dataflow/external-models/validatemodels.expected +++ b/cpp/ql/test/library-tests/dataflow/external-models/validatemodels.expected @@ -16,6 +16,7 @@ | Dubious signature "(const_iterator,size_type,const T &)" in summary model. | | Dubious signature "(deque &&)" in summary model. | | Dubious signature "(deque &&,const Allocator &)" in summary model. | +| Dubious signature "(format_string,Args &&)" in summary model. | | Dubious signature "(forward_list &&)" in summary model. | | Dubious signature "(forward_list &&,const Allocator &)" in summary model. | | Dubious signature "(list &&)" in summary model. | From ef0370b64e80b60b34e0f3c00889dd70702b13a2 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 30 Jul 2024 13:19:02 +0100 Subject: [PATCH 8/9] C++: Respond to review comments. --- cpp/ql/lib/ext/std.format.model.yml | 17 +++++++++-------- .../dataflow/taint-tests/format.cpp | 3 +++ .../dataflow/taint-tests/localTaint.expected | 2 ++ .../taint-tests/test_mad-signatures.expected | 5 +++++ 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/cpp/ql/lib/ext/std.format.model.yml b/cpp/ql/lib/ext/std.format.model.yml index dbd547009556..229de2e48666 100644 --- a/cpp/ql/lib/ext/std.format.model.yml +++ b/cpp/ql/lib/ext/std.format.model.yml @@ -3,11 +3,12 @@ extensions: pack: codeql/cpp-all extensible: summaryModel data: # namespace, type, subtypes, name, signature, ext, input, output, kind, provenance - - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*1]", "ReturnValue.Element[@]", "taint", "manual"] - - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*2]", "ReturnValue.Element[@]", "taint", "manual"] - - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*3]", "ReturnValue.Element[@]", "taint", "manual"] - - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*4]", "ReturnValue.Element[@]", "taint", "manual"] - - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*5]", "ReturnValue.Element[@]", "taint", "manual"] - - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*6]", "ReturnValue.Element[@]", "taint", "manual"] - - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*7]", "ReturnValue.Element[@]", "taint", "manual"] - - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*8]", "ReturnValue.Element[@]", "taint", "manual"] \ No newline at end of file + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[0]", "ReturnValue", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*1]", "ReturnValue.Element", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*2]", "ReturnValue.Element", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*3]", "ReturnValue.Element", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*4]", "ReturnValue.Element", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*5]", "ReturnValue.Element", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*6]", "ReturnValue.Element", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*7]", "ReturnValue.Element", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*8]", "ReturnValue.Element", "taint", "manual"] \ No newline at end of file diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/format.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/format.cpp index a5cfadea464b..d123ed5cc155 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/format.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/format.cpp @@ -161,4 +161,7 @@ void test2() void test_format() { auto s = std::format("{}", string::source()); sink(s); // $ ir MISSING: ast + + auto s2 = std::format(string::source()); + sink(s2); // $ ir MISSING: ast } \ No newline at end of file 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 d565f6d80e08..c6c5e86635d9 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected @@ -449,6 +449,8 @@ WARNING: module 'TaintTracking' has been deprecated and may be removed in future | format.cpp:158:26:158:26 | 2 | format.cpp:158:13:158:26 | ... / ... | TAINT | | format.cpp:162:12:162:22 | call to format | format.cpp:163:8:163:8 | s | | | format.cpp:162:24:162:27 | {} | format.cpp:162:24:162:27 | call to basic_format_string | TAINT | +| format.cpp:165:13:165:23 | call to format | format.cpp:166:8:166:9 | s2 | | +| format.cpp:165:25:165:38 | call to source | format.cpp:165:25:165:40 | call to basic_format_string | TAINT | | map.cpp:21:28:21:28 | call to pair | map.cpp:23:2:23:2 | a | | | map.cpp:21:28:21:28 | call to pair | map.cpp:24:7:24:7 | a | | | map.cpp:21:28:21:28 | call to pair | map.cpp:25:7:25:7 | a | | diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.expected index 209a02094fef..0d27fdaf0fda 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.expected @@ -262,6 +262,8 @@ signatureMatches | stl.h:628:38:628:43 | insert | (InputIt,InputIt) | vector | assign | 0 | | stl.h:628:38:628:43 | insert | (InputIt,InputIt) | vector | assign | 1 | | stl.h:678:33:678:38 | format | (format_string,Args &&) | | format | 0 | +| stl.h:678:33:678:38 | format | (format_string,Args &&) | | format | 0 | +| stl.h:678:33:678:38 | format | (format_string,Args &&) | | format | 1 | | stl.h:678:33:678:38 | format | (format_string,Args &&) | | format | 1 | getSignatureParameterName | (InputIt,InputIt) | deque | assign | 0 | func:0 | @@ -722,7 +724,10 @@ getParameterTypeName | stl.h:639:12:639:15 | find | 0 | const key_type & | | stl.h:641:28:641:38 | equal_range | 0 | const key_type & | | stl.h:671:21:671:39 | basic_format_string | 0 | const func:0 & | +| stl.h:671:21:671:39 | basic_format_string | 0 | const func:0 & | | stl.h:678:33:678:38 | format | 0 | format_string | +| stl.h:678:33:678:38 | format | 0 | format_string | +| stl.h:678:33:678:38 | format | 1 | func:0 && | | stl.h:678:33:678:38 | format | 1 | func:0 && | | stringstream.cpp:18:6:18:9 | sink | 0 | const basic_ostream> & | | stringstream.cpp:21:6:21:9 | sink | 0 | const basic_istream> & | From 6d8a83fc1f161922c37e5ebe78b4202397f5fae3 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 30 Jul 2024 15:31:18 +0100 Subject: [PATCH 9/9] C++: Take out the 'Element' content from std::format model. --- cpp/ql/lib/ext/std.format.model.yml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cpp/ql/lib/ext/std.format.model.yml b/cpp/ql/lib/ext/std.format.model.yml index 229de2e48666..45136213cc48 100644 --- a/cpp/ql/lib/ext/std.format.model.yml +++ b/cpp/ql/lib/ext/std.format.model.yml @@ -4,11 +4,11 @@ extensions: extensible: summaryModel data: # namespace, type, subtypes, name, signature, ext, input, output, kind, provenance - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[0]", "ReturnValue", "taint", "manual"] - - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*1]", "ReturnValue.Element", "taint", "manual"] - - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*2]", "ReturnValue.Element", "taint", "manual"] - - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*3]", "ReturnValue.Element", "taint", "manual"] - - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*4]", "ReturnValue.Element", "taint", "manual"] - - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*5]", "ReturnValue.Element", "taint", "manual"] - - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*6]", "ReturnValue.Element", "taint", "manual"] - - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*7]", "ReturnValue.Element", "taint", "manual"] - - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*8]", "ReturnValue.Element", "taint", "manual"] \ No newline at end of file + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*1]", "ReturnValue", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*2]", "ReturnValue", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*3]", "ReturnValue", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*4]", "ReturnValue", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*5]", "ReturnValue", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*6]", "ReturnValue", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*7]", "ReturnValue", "taint", "manual"] + - ["std", "", False, "format", "(format_string,Args &&)", "", "Argument[*8]", "ReturnValue", "taint", "manual"] \ No newline at end of file