From f132bca06e925f4e3335983072ff3f8cbec53e5b Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Fri, 12 Jul 2019 16:17:52 +0100 Subject: [PATCH 1/3] CPP: Add a taint flow test of 'std::swap'. --- .../dataflow/taint-tests/localTaint.expected | 12 +++++++++++ .../dataflow/taint-tests/taint.cpp | 21 +++++++++++++++++++ .../dataflow/taint-tests/taint.expected | 2 ++ .../dataflow/taint-tests/test_diff.expected | 1 + .../dataflow/taint-tests/test_ir.expected | 1 + 5 files changed, 37 insertions(+) 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 7dd0cec2b86..b228b2d47af 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected @@ -161,3 +161,15 @@ | taint.cpp:194:10:194:10 | x | taint.cpp:194:9:194:10 | & ... | TAINT | | taint.cpp:194:13:194:18 | source | taint.cpp:194:9:194:10 | ref arg & ... | TAINT | | taint.cpp:194:21:194:31 | sizeof(int) | taint.cpp:194:9:194:10 | ref arg & ... | TAINT | +| taint.cpp:207:6:207:11 | call to source | taint.cpp:207:2:207:13 | ... = ... | | +| taint.cpp:207:6:207:11 | call to source | taint.cpp:210:7:210:7 | x | | +| taint.cpp:207:6:207:11 | call to source | taint.cpp:213:12:213:12 | x | | +| taint.cpp:207:6:207:11 | call to source | taint.cpp:215:7:215:7 | x | | +| taint.cpp:208:6:208:6 | 0 | taint.cpp:208:2:208:6 | ... = ... | | +| taint.cpp:208:6:208:6 | 0 | taint.cpp:211:7:211:7 | y | | +| taint.cpp:208:6:208:6 | 0 | taint.cpp:213:15:213:15 | y | | +| taint.cpp:208:6:208:6 | 0 | taint.cpp:216:7:216:7 | y | | +| taint.cpp:213:12:213:12 | ref arg x | taint.cpp:213:12:213:12 | x | | +| taint.cpp:213:12:213:12 | ref arg x | taint.cpp:215:7:215:7 | x | | +| taint.cpp:213:15:213:15 | ref arg y | taint.cpp:213:15:213:15 | y | | +| taint.cpp:213:15:213:15 | ref arg y | taint.cpp:216:7:216:7 | y | | 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 fc0912cb9df..b2ad4f790be 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp @@ -194,3 +194,24 @@ void test_memcpy(int *source) { memcpy(&x, source, sizeof(int)); sink(x); } + +// --- swap --- + +namespace std { + template constexpr void swap(T& a, T& b); +} + +void test_swap() { + int x, y; + + x = source(); + y = 0; + + sink(x); // tainted + sink(y); + + std::swap(x, y); + + sink(x); // [FALSE POSITIVE] + sink(y); // tainted [NOT DETECTED] +} diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected index 93bc8084c87..f0e101127e5 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected @@ -14,3 +14,5 @@ | taint.cpp:181:8:181:9 | * ... | taint.cpp:185:11:185:16 | call to source | | taint.cpp:195:7:195:7 | x | taint.cpp:192:23:192:28 | source | | taint.cpp:195:7:195:7 | x | taint.cpp:193:6:193:6 | x | +| taint.cpp:210:7:210:7 | x | taint.cpp:207:6:207:11 | call to source | +| taint.cpp:215:7:215:7 | x | taint.cpp:207:6:207:11 | call to source | diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected index cce0efdcfc8..4cf348d9b37 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected @@ -7,3 +7,4 @@ | taint.cpp:185:11:185:16 | taint.cpp:181:8:181:9 | AST only | | taint.cpp:192:23:192:28 | taint.cpp:195:7:195:7 | AST only | | taint.cpp:193:6:193:6 | taint.cpp:195:7:195:7 | AST only | +| taint.cpp:207:6:207:11 | taint.cpp:215:7:215:7 | AST only | diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.expected index aa3094506da..6c2cbe0617f 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.expected @@ -7,3 +7,4 @@ | taint.cpp:151:7:151:12 | Call: call to select | taint.cpp:151:20:151:25 | Call: call to source | | taint.cpp:167:8:167:13 | Call: call to source | taint.cpp:167:8:167:13 | Call: call to source | | taint.cpp:168:8:168:14 | Load: tainted | taint.cpp:164:19:164:24 | Call: call to source | +| taint.cpp:210:7:210:7 | Load: x | taint.cpp:207:6:207:11 | Call: call to source | From c2fd2e273ea590ef089ab604cd1dedbf823facce Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Fri, 12 Jul 2019 17:06:44 +0100 Subject: [PATCH 2/3] CPP: Model taint flow through std::swap. --- cpp/ql/src/semmle/code/cpp/models/Models.qll | 1 + .../code/cpp/models/implementations/Swap.qll | 23 +++++++++++++++++++ .../dataflow/taint-tests/localTaint.expected | 2 ++ .../dataflow/taint-tests/taint.cpp | 2 +- .../dataflow/taint-tests/taint.expected | 1 + .../dataflow/taint-tests/test_diff.expected | 1 + 6 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 cpp/ql/src/semmle/code/cpp/models/implementations/Swap.qll diff --git a/cpp/ql/src/semmle/code/cpp/models/Models.qll b/cpp/ql/src/semmle/code/cpp/models/Models.qll index 71526b8fca2..0747b00c48d 100644 --- a/cpp/ql/src/semmle/code/cpp/models/Models.qll +++ b/cpp/ql/src/semmle/code/cpp/models/Models.qll @@ -6,3 +6,4 @@ private import implementations.Pure private import implementations.Strcat private import implementations.Strcpy private import implementations.Strftime +private import implementations.Swap diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Swap.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Swap.qll new file mode 100644 index 00000000000..ec8f963fdbf --- /dev/null +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Swap.qll @@ -0,0 +1,23 @@ +import semmle.code.cpp.models.interfaces.DataFlow +import semmle.code.cpp.models.interfaces.Taint + +/** + * The standard function `swap`. + */ +class Swap extends DataFlowFunction { + Swap() { + this.hasQualifiedName("std", "swap") + } + + override predicate hasDataFlow(FunctionInput input, FunctionOutput output) { + ( + input.isInParameterPointer(0) and + output.isOutParameterPointer(1) + ) + or + ( + input.isInParameterPointer(1) and + output.isOutParameterPointer(0) + ) + } +} 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 b228b2d47af..ac9d34d7cfd 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected @@ -171,5 +171,7 @@ | taint.cpp:208:6:208:6 | 0 | taint.cpp:216:7:216:7 | y | | | taint.cpp:213:12:213:12 | ref arg x | taint.cpp:213:12:213:12 | x | | | taint.cpp:213:12:213:12 | ref arg x | taint.cpp:215:7:215:7 | x | | +| taint.cpp:213:12:213:12 | x | taint.cpp:213:15:213:15 | ref arg y | | | taint.cpp:213:15:213:15 | ref arg y | taint.cpp:213:15:213:15 | y | | | taint.cpp:213:15:213:15 | ref arg y | taint.cpp:216:7:216:7 | y | | +| taint.cpp:213:15:213:15 | y | taint.cpp:213:12:213:12 | ref arg x | | 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 b2ad4f790be..90d043dd45b 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp @@ -213,5 +213,5 @@ void test_swap() { std::swap(x, y); sink(x); // [FALSE POSITIVE] - sink(y); // tainted [NOT DETECTED] + sink(y); // tainted } diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected index f0e101127e5..dd03a3b49c5 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected @@ -16,3 +16,4 @@ | taint.cpp:195:7:195:7 | x | taint.cpp:193:6:193:6 | x | | taint.cpp:210:7:210:7 | x | taint.cpp:207:6:207:11 | call to source | | taint.cpp:215:7:215:7 | x | taint.cpp:207:6:207:11 | call to source | +| taint.cpp:216:7:216:7 | y | taint.cpp:207:6:207:11 | call to source | diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected index 4cf348d9b37..5718971a4ac 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected @@ -8,3 +8,4 @@ | taint.cpp:192:23:192:28 | taint.cpp:195:7:195:7 | AST only | | taint.cpp:193:6:193:6 | taint.cpp:195:7:195:7 | AST only | | taint.cpp:207:6:207:11 | taint.cpp:215:7:215:7 | AST only | +| taint.cpp:207:6:207:11 | taint.cpp:216:7:216:7 | AST only | From cd449e13363143bae5474540e660d9e6bbbc2687 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Fri, 12 Jul 2019 17:40:31 +0100 Subject: [PATCH 3/3] CPP: Change note. --- change-notes/1.22/analysis-cpp.md | 1 + 1 file changed, 1 insertion(+) diff --git a/change-notes/1.22/analysis-cpp.md b/change-notes/1.22/analysis-cpp.md index 19bb2898ec0..5d26b89a7c4 100644 --- a/change-notes/1.22/analysis-cpp.md +++ b/change-notes/1.22/analysis-cpp.md @@ -22,3 +22,4 @@ - The predicate `TypeMention.toString()` has been simplified to always return the string "`type mention`". This may improve performance when using `Element.toString()` or its descendants. - The `semmle.code.cpp.security.TaintTracking` library now considers a pointer difference calculation as blocking taint flow. - Fixed the `LocalScopeVariableReachability.qll` library's handling of loops with an entry condition is both always true upon first entry, and where there is more than one control flow path through the loop condition. This change increases the accuracy of the `LocalScopeVariableReachability.qll` library and queries which depend on it. +- The `semmle.code.cpp.models` library now models data flow through `std::swap`.