From 56390c1aef542fccf5838e5f2739dfff5f7d8a09 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Fri, 11 Sep 2020 13:41:36 +0100 Subject: [PATCH 1/7] C++: Flow through operator>>. --- .../cpp/models/implementations/StdString.qll | 57 +++++++++++++++++++ .../dataflow/taint-tests/localTaint.expected | 22 +++++++ .../dataflow/taint-tests/stringstream.cpp | 24 ++++---- .../dataflow/taint-tests/taint.expected | 12 ++++ .../dataflow/taint-tests/test_diff.expected | 12 ++++ .../dataflow/taint-tests/test_ir.expected | 8 +++ 6 files changed, 123 insertions(+), 12 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll index f5fc470a42c..f62b550b46d 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll @@ -288,6 +288,63 @@ class StdStringAt extends TaintFunction { } } +/** + * The `std::basic_istream` template class. + */ +class StdBasicIStream extends TemplateClass { + StdBasicIStream() { this.hasQualifiedName("std", "basic_istream") } +} + +/** + * The `std::istream` function `operator>>` (defined as a member function). + */ +class StdIStreamIn extends DataFlowFunction, TaintFunction { + StdIStreamIn() { this.hasQualifiedName("std", "basic_istream", "operator>>") } + + override predicate hasDataFlow(FunctionInput input, FunctionOutput output) { + // flow from qualifier to return value + input.isQualifierObject() and + output.isReturnValue() + } + + override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { + // flow from qualifier to first parameter + input.isQualifierObject() and + output.isParameterDeref(0) + or + // reverse flow from returned reference to the qualifier + input.isReturnValueDeref() and + output.isQualifierObject() + } +} + +/** + * The `std::istream` function `operator>>` (defined as a non-member function). + */ +class StdIStreamInNonMember extends DataFlowFunction, TaintFunction { + StdIStreamInNonMember() { + this.hasQualifiedName("std", "operator>>") and + this.getUnspecifiedType().(ReferenceType).getBaseType() = + any(StdBasicIStream s).getAnInstantiation() + } + + override predicate hasDataFlow(FunctionInput input, FunctionOutput output) { + // flow from first parameter to return value + input.isParameter(0) and + output.isReturnValue() + } + + override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { + // flow from first parameter to second parameter + input.isParameterDeref(0) and + output.isParameterDeref(1) + or + // reverse flow from returned reference to the first parameter + input.isReturnValueDeref() and + output.isParameterDeref(0) + } +} + /** * The `std::basic_ostream` template class. */ 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 3173ddf985e..f615acb4b05 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected @@ -1568,9 +1568,13 @@ | stringstream.cpp:76:14:76:19 | source | stringstream.cpp:76:11:76:11 | call to operator<< | TAINT | | stringstream.cpp:77:7:77:9 | ref arg ss1 | stringstream.cpp:80:7:80:9 | ss1 | | | stringstream.cpp:77:7:77:9 | ref arg ss1 | stringstream.cpp:82:7:82:9 | ss1 | | +| stringstream.cpp:77:7:77:9 | ss1 | stringstream.cpp:77:11:77:11 | call to operator>> | | +| stringstream.cpp:77:7:77:9 | ss1 | stringstream.cpp:77:14:77:15 | ref arg v1 | TAINT | | stringstream.cpp:77:14:77:15 | ref arg v1 | stringstream.cpp:84:7:84:8 | v1 | | | stringstream.cpp:78:7:78:9 | ref arg ss2 | stringstream.cpp:81:7:81:9 | ss2 | | | stringstream.cpp:78:7:78:9 | ref arg ss2 | stringstream.cpp:83:7:83:9 | ss2 | | +| stringstream.cpp:78:7:78:9 | ss2 | stringstream.cpp:78:11:78:11 | call to operator>> | | +| stringstream.cpp:78:7:78:9 | ss2 | stringstream.cpp:78:14:78:15 | ref arg v2 | TAINT | | stringstream.cpp:78:14:78:15 | ref arg v2 | stringstream.cpp:85:7:85:8 | v2 | | | stringstream.cpp:82:7:82:9 | ss1 | stringstream.cpp:82:11:82:13 | call to str | TAINT | | stringstream.cpp:83:7:83:9 | ss2 | stringstream.cpp:83:11:83:13 | call to str | TAINT | @@ -1715,6 +1719,8 @@ | stringstream.cpp:145:7:145:9 | ref arg ss1 | stringstream.cpp:174:12:174:14 | ss1 | | | stringstream.cpp:145:7:145:9 | ref arg ss1 | stringstream.cpp:176:12:176:14 | ss1 | | | stringstream.cpp:145:7:145:9 | ref arg ss1 | stringstream.cpp:178:7:178:9 | ss1 | | +| stringstream.cpp:145:7:145:9 | ss1 | stringstream.cpp:145:11:145:11 | call to operator>> | | +| stringstream.cpp:145:7:145:9 | ss1 | stringstream.cpp:145:14:145:15 | ref arg s1 | TAINT | | stringstream.cpp:145:14:145:15 | ref arg s1 | stringstream.cpp:148:7:148:8 | s1 | | | stringstream.cpp:146:7:146:9 | ref arg ss2 | stringstream.cpp:147:7:147:9 | ss2 | | | stringstream.cpp:146:7:146:9 | ref arg ss2 | stringstream.cpp:154:7:154:9 | ss2 | | @@ -1725,6 +1731,8 @@ | stringstream.cpp:146:7:146:9 | ref arg ss2 | stringstream.cpp:175:12:175:14 | ss2 | | | stringstream.cpp:146:7:146:9 | ref arg ss2 | stringstream.cpp:177:12:177:14 | ss2 | | | stringstream.cpp:146:7:146:9 | ref arg ss2 | stringstream.cpp:179:7:179:9 | ss2 | | +| stringstream.cpp:146:7:146:9 | ss2 | stringstream.cpp:146:11:146:11 | call to operator>> | | +| stringstream.cpp:146:7:146:9 | ss2 | stringstream.cpp:146:14:146:15 | ref arg s2 | TAINT | | stringstream.cpp:146:14:146:15 | ref arg s2 | stringstream.cpp:149:7:149:8 | s2 | | | stringstream.cpp:147:7:147:9 | ref arg ss2 | stringstream.cpp:154:7:154:9 | ss2 | | | stringstream.cpp:147:7:147:9 | ref arg ss2 | stringstream.cpp:155:7:155:9 | ss2 | | @@ -1734,6 +1742,11 @@ | stringstream.cpp:147:7:147:9 | ref arg ss2 | stringstream.cpp:175:12:175:14 | ss2 | | | stringstream.cpp:147:7:147:9 | ref arg ss2 | stringstream.cpp:177:12:177:14 | ss2 | | | stringstream.cpp:147:7:147:9 | ref arg ss2 | stringstream.cpp:179:7:179:9 | ss2 | | +| stringstream.cpp:147:7:147:9 | ss2 | stringstream.cpp:147:11:147:11 | call to operator>> | | +| stringstream.cpp:147:7:147:9 | ss2 | stringstream.cpp:147:14:147:15 | ref arg s3 | TAINT | +| stringstream.cpp:147:11:147:11 | call to operator>> | stringstream.cpp:147:17:147:17 | call to operator>> | | +| stringstream.cpp:147:11:147:11 | call to operator>> | stringstream.cpp:147:20:147:21 | ref arg s4 | TAINT | +| stringstream.cpp:147:11:147:11 | ref arg call to operator>> | stringstream.cpp:147:7:147:9 | ref arg ss2 | TAINT | | stringstream.cpp:147:14:147:15 | ref arg s3 | stringstream.cpp:150:7:150:8 | s3 | | | stringstream.cpp:147:20:147:21 | ref arg s4 | stringstream.cpp:151:7:151:8 | s4 | | | stringstream.cpp:153:7:153:9 | ref arg ss1 | stringstream.cpp:161:7:161:9 | ss1 | | @@ -1742,6 +1755,8 @@ | stringstream.cpp:153:7:153:9 | ref arg ss1 | stringstream.cpp:174:12:174:14 | ss1 | | | stringstream.cpp:153:7:153:9 | ref arg ss1 | stringstream.cpp:176:12:176:14 | ss1 | | | stringstream.cpp:153:7:153:9 | ref arg ss1 | stringstream.cpp:178:7:178:9 | ss1 | | +| stringstream.cpp:153:7:153:9 | ss1 | stringstream.cpp:153:11:153:11 | call to operator>> | | +| stringstream.cpp:153:7:153:9 | ss1 | stringstream.cpp:153:14:153:15 | ref arg b1 | TAINT | | stringstream.cpp:153:14:153:15 | ref arg b1 | stringstream.cpp:156:7:156:8 | b1 | | | stringstream.cpp:154:7:154:9 | ref arg ss2 | stringstream.cpp:155:7:155:9 | ss2 | | | stringstream.cpp:154:7:154:9 | ref arg ss2 | stringstream.cpp:162:7:162:9 | ss2 | | @@ -1750,6 +1765,8 @@ | stringstream.cpp:154:7:154:9 | ref arg ss2 | stringstream.cpp:175:12:175:14 | ss2 | | | stringstream.cpp:154:7:154:9 | ref arg ss2 | stringstream.cpp:177:12:177:14 | ss2 | | | stringstream.cpp:154:7:154:9 | ref arg ss2 | stringstream.cpp:179:7:179:9 | ss2 | | +| stringstream.cpp:154:7:154:9 | ss2 | stringstream.cpp:154:11:154:11 | call to operator>> | | +| stringstream.cpp:154:7:154:9 | ss2 | stringstream.cpp:154:14:154:15 | ref arg b2 | TAINT | | stringstream.cpp:154:14:154:15 | ref arg b2 | stringstream.cpp:157:7:157:8 | b2 | | | stringstream.cpp:155:7:155:9 | ref arg ss2 | stringstream.cpp:162:7:162:9 | ss2 | | | stringstream.cpp:155:7:155:9 | ref arg ss2 | stringstream.cpp:164:7:164:9 | ss2 | | @@ -1757,6 +1774,11 @@ | stringstream.cpp:155:7:155:9 | ref arg ss2 | stringstream.cpp:175:12:175:14 | ss2 | | | stringstream.cpp:155:7:155:9 | ref arg ss2 | stringstream.cpp:177:12:177:14 | ss2 | | | stringstream.cpp:155:7:155:9 | ref arg ss2 | stringstream.cpp:179:7:179:9 | ss2 | | +| stringstream.cpp:155:7:155:9 | ss2 | stringstream.cpp:155:11:155:11 | call to operator>> | | +| stringstream.cpp:155:7:155:9 | ss2 | stringstream.cpp:155:14:155:15 | ref arg b3 | TAINT | +| stringstream.cpp:155:11:155:11 | call to operator>> | stringstream.cpp:155:17:155:17 | call to operator>> | | +| stringstream.cpp:155:11:155:11 | call to operator>> | stringstream.cpp:155:20:155:21 | ref arg b4 | TAINT | +| stringstream.cpp:155:11:155:11 | ref arg call to operator>> | stringstream.cpp:155:7:155:9 | ref arg ss2 | TAINT | | stringstream.cpp:155:14:155:15 | ref arg b3 | stringstream.cpp:158:7:158:8 | b3 | | | stringstream.cpp:155:20:155:21 | ref arg b4 | stringstream.cpp:159:7:159:8 | b4 | | | stringstream.cpp:156:7:156:8 | b1 | stringstream.cpp:156:7:156:8 | call to basic_string | TAINT | diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp index d9b35b4a9e4..6f5102a8270 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp @@ -75,14 +75,14 @@ void test_stringstream_int(int source) sink(ss1 << 1234); sink(ss2 << source); // tainted sink(ss1 >> v1); - sink(ss2 >> v2); // tainted [NOT DETECTED] + sink(ss2 >> v2); // tainted sink(ss1); sink(ss2); // tainted sink(ss1.str()); sink(ss2.str()); // tainted sink(v1); - sink(v2); // tainted [NOT DETECTED] + sink(v2); // tainted } void test_stringstream_constructors() @@ -143,20 +143,20 @@ void test_stringstream_in() sink(ss2 << source()); // tainted sink(ss1 >> s1); - sink(ss2 >> s2); // tainted [NOT DETECTED] - sink(ss2 >> s3 >> s4); // tainted [NOT DETECTED] + sink(ss2 >> s2); // tainted + sink(ss2 >> s3 >> s4); // tainted sink(s1); - sink(s2); // tainted [NOT DETECTED] - sink(s3); // tainted [NOT DETECTED] - sink(s4); // tainted [NOT DETECTED] + sink(s2); // tainted + sink(s3); // tainted + sink(s4); // tainted sink(ss1 >> b1); - sink(ss2 >> b2); - sink(ss2 >> b3 >> b4); + sink(ss2 >> b2); // tainted + sink(ss2 >> b3 >> b4); // tainted sink(b1); - sink(b2); // tainted [NOT DETECTED] - sink(b3); // tainted [NOT DETECTED] - sink(b4); // tainted [NOT DETECTED] + sink(b2); // tainted + sink(b3); // tainted + sink(b4); // tainted sink(ss1.read(b5, 100)); sink(ss2.read(b6, 100)); // 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 c8a4c9cc189..8ed02e04295 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected @@ -188,8 +188,10 @@ | stringstream.cpp:66:7:66:10 | ss12 | stringstream.cpp:63:18:63:23 | call to source | | stringstream.cpp:67:7:67:10 | ss13 | stringstream.cpp:64:36:64:41 | call to source | | stringstream.cpp:76:11:76:11 | call to operator<< | stringstream.cpp:70:32:70:37 | source | +| stringstream.cpp:78:11:78:11 | call to operator>> | stringstream.cpp:70:32:70:37 | source | | stringstream.cpp:81:7:81:9 | ss2 | stringstream.cpp:70:32:70:37 | source | | stringstream.cpp:83:11:83:13 | call to str | stringstream.cpp:70:32:70:37 | source | +| stringstream.cpp:85:7:85:8 | v2 | stringstream.cpp:70:32:70:37 | source | | stringstream.cpp:100:11:100:11 | call to operator= | stringstream.cpp:100:31:100:36 | call to source | | stringstream.cpp:103:7:103:9 | ss2 | stringstream.cpp:91:19:91:24 | call to source | | stringstream.cpp:105:7:105:9 | ss4 | stringstream.cpp:95:44:95:49 | call to source | @@ -197,6 +199,16 @@ | stringstream.cpp:121:7:121:9 | ss2 | stringstream.cpp:113:24:113:29 | call to source | | stringstream.cpp:123:7:123:9 | ss4 | stringstream.cpp:115:24:115:29 | call to source | | stringstream.cpp:143:11:143:11 | call to operator<< | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:146:11:146:11 | call to operator>> | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:147:17:147:17 | call to operator>> | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:149:7:149:8 | s2 | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:150:7:150:8 | s3 | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:151:7:151:8 | s4 | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:154:11:154:11 | call to operator>> | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:155:17:155:17 | call to operator>> | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:157:7:157:8 | call to basic_string | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:158:7:158:8 | call to basic_string | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:159:7:159:8 | call to basic_string | stringstream.cpp:143:14:143:19 | call to source | | structlikeclass.cpp:35:8:35:9 | s1 | structlikeclass.cpp:29:22:29:27 | call to source | | structlikeclass.cpp:36:8:36:9 | s2 | structlikeclass.cpp:30:24:30:29 | call to source | | structlikeclass.cpp:37:8:37:9 | s3 | structlikeclass.cpp:29:22:29:27 | 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 6698c02c81c..e8bc4f0ad4a 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 @@ -188,8 +188,10 @@ | stringstream.cpp:66:7:66:10 | stringstream.cpp:63:18:63:23 | AST only | | stringstream.cpp:67:7:67:10 | stringstream.cpp:64:36:64:41 | AST only | | stringstream.cpp:76:11:76:11 | stringstream.cpp:70:32:70:37 | AST only | +| stringstream.cpp:78:11:78:11 | stringstream.cpp:70:32:70:37 | AST only | | stringstream.cpp:81:7:81:9 | stringstream.cpp:70:32:70:37 | AST only | | stringstream.cpp:83:11:83:13 | stringstream.cpp:70:32:70:37 | AST only | +| stringstream.cpp:85:7:85:8 | stringstream.cpp:70:32:70:37 | AST only | | stringstream.cpp:100:11:100:11 | stringstream.cpp:100:31:100:36 | AST only | | stringstream.cpp:103:7:103:9 | stringstream.cpp:91:19:91:24 | AST only | | stringstream.cpp:105:7:105:9 | stringstream.cpp:95:44:95:49 | AST only | @@ -199,6 +201,16 @@ | stringstream.cpp:143:11:143:11 | stringstream.cpp:143:14:143:21 | IR only | | stringstream.cpp:143:11:143:22 | stringstream.cpp:143:14:143:19 | IR only | | stringstream.cpp:143:11:143:22 | stringstream.cpp:143:14:143:21 | IR only | +| stringstream.cpp:146:11:146:11 | stringstream.cpp:143:14:143:19 | AST only | +| stringstream.cpp:147:17:147:17 | stringstream.cpp:143:14:143:19 | AST only | +| stringstream.cpp:149:7:149:8 | stringstream.cpp:143:14:143:19 | AST only | +| stringstream.cpp:150:7:150:8 | stringstream.cpp:143:14:143:19 | AST only | +| stringstream.cpp:151:7:151:8 | stringstream.cpp:143:14:143:19 | AST only | +| stringstream.cpp:154:11:154:11 | stringstream.cpp:143:14:143:19 | AST only | +| stringstream.cpp:155:17:155:17 | stringstream.cpp:143:14:143:19 | AST only | +| stringstream.cpp:157:7:157:8 | stringstream.cpp:143:14:143:21 | IR only | +| stringstream.cpp:158:7:158:8 | stringstream.cpp:143:14:143:21 | IR only | +| stringstream.cpp:159:7:159:8 | stringstream.cpp:143:14:143:19 | AST only | | swap1.cpp:78:12:78:16 | swap1.cpp:69:23:69:23 | AST only | | swap1.cpp:87:13:87:17 | swap1.cpp:82:16:82:21 | AST only | | swap1.cpp:88:13:88:17 | swap1.cpp:81:27:81:28 | 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 c2c9b35bd4b..b299b4bab9c 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 @@ -70,6 +70,14 @@ | stringstream.cpp:143:11:143:22 | (reference dereference) | stringstream.cpp:143:14:143:21 | (const char *)... | | stringstream.cpp:143:11:143:22 | (reference to) | stringstream.cpp:143:14:143:19 | call to source | | stringstream.cpp:143:11:143:22 | (reference to) | stringstream.cpp:143:14:143:21 | (const char *)... | +| stringstream.cpp:157:7:157:8 | (reference to) | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:157:7:157:8 | (reference to) | stringstream.cpp:143:14:143:21 | (const char *)... | +| stringstream.cpp:157:7:157:8 | call to basic_string | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:157:7:157:8 | call to basic_string | stringstream.cpp:143:14:143:21 | (const char *)... | +| stringstream.cpp:158:7:158:8 | (reference to) | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:158:7:158:8 | (reference to) | stringstream.cpp:143:14:143:21 | (const char *)... | +| stringstream.cpp:158:7:158:8 | call to basic_string | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:158:7:158:8 | call to basic_string | stringstream.cpp:143:14:143:21 | (const char *)... | | structlikeclass.cpp:35:8:35:9 | s1 | structlikeclass.cpp:29:22:29:27 | call to source | | structlikeclass.cpp:36:8:36:9 | s2 | structlikeclass.cpp:30:24:30:29 | call to source | | structlikeclass.cpp:37:8:37:9 | s3 | structlikeclass.cpp:29:22:29:27 | call to source | From 7cc60a30a6070739e991c732104db6ee8ce09cc9 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Mon, 14 Sep 2020 10:58:05 +0100 Subject: [PATCH 2/7] C++: Flow through get, peek, read, readsome. --- .../cpp/models/implementations/StdString.qll | 55 +++++++++++++++++++ .../dataflow/taint-tests/localTaint.expected | 21 +++++++ .../dataflow/taint-tests/stringstream.cpp | 22 ++++---- .../dataflow/taint-tests/taint.expected | 11 ++++ .../dataflow/taint-tests/test_diff.expected | 11 ++++ 5 files changed, 109 insertions(+), 11 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll index f62b550b46d..5797a76acc6 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll @@ -345,6 +345,61 @@ class StdIStreamInNonMember extends DataFlowFunction, TaintFunction { } } +/** + * The `std::istream` functions `get` (without parameters) and `peek`. + */ +class StdIStreamGet extends TaintFunction { + StdIStreamGet() { + this.hasQualifiedName("std", "basic_istream", ["get", "peek"]) and + this.getNumberOfParameters() = 0 + } + + override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { + // flow from qualifier to return value + input.isQualifierObject() and + output.isReturnValue() + } +} + +/** + * The `std::istream` functions `get` (with parameters) and `read`. + */ +class StdIStreamRead extends DataFlowFunction, TaintFunction { + StdIStreamRead() { + this.hasQualifiedName("std", "basic_istream", ["get", "read"]) and + this.getNumberOfParameters() > 0 + } + + override predicate hasDataFlow(FunctionInput input, FunctionOutput output) { + // flow from qualifier to return value + input.isQualifierObject() and + output.isReturnValue() + } + + override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { + // flow from qualifier to first parameter + input.isQualifierObject() and + output.isParameterDeref(0) + or + // reverse flow from returned reference to the qualifier + input.isReturnValueDeref() and + output.isQualifierObject() + } +} + +/** + * The `std::istream` function `readsome`. + */ +class StdIStreamReadSome extends TaintFunction { + StdIStreamReadSome() { this.hasQualifiedName("std", "basic_istream", "readsome") } + + override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { + // flow from qualifier to first parameter + input.isQualifierObject() and + output.isParameterDeref(0) + } +} + /** * The `std::basic_ostream` template class. */ 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 f615acb4b05..a3ed313206c 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected @@ -1790,30 +1790,40 @@ | stringstream.cpp:161:7:161:9 | ref arg ss1 | stringstream.cpp:174:12:174:14 | ss1 | | | stringstream.cpp:161:7:161:9 | ref arg ss1 | stringstream.cpp:176:12:176:14 | ss1 | | | stringstream.cpp:161:7:161:9 | ref arg ss1 | stringstream.cpp:178:7:178:9 | ss1 | | +| stringstream.cpp:161:7:161:9 | ss1 | stringstream.cpp:161:11:161:14 | call to read | | +| stringstream.cpp:161:7:161:9 | ss1 | stringstream.cpp:161:16:161:17 | ref arg b5 | TAINT | | stringstream.cpp:161:16:161:17 | ref arg b5 | stringstream.cpp:167:7:167:8 | b5 | | | stringstream.cpp:162:7:162:9 | ref arg ss2 | stringstream.cpp:164:7:164:9 | ss2 | | | stringstream.cpp:162:7:162:9 | ref arg ss2 | stringstream.cpp:166:7:166:9 | ss2 | | | stringstream.cpp:162:7:162:9 | ref arg ss2 | stringstream.cpp:175:12:175:14 | ss2 | | | stringstream.cpp:162:7:162:9 | ref arg ss2 | stringstream.cpp:177:12:177:14 | ss2 | | | stringstream.cpp:162:7:162:9 | ref arg ss2 | stringstream.cpp:179:7:179:9 | ss2 | | +| stringstream.cpp:162:7:162:9 | ss2 | stringstream.cpp:162:11:162:14 | call to read | | +| stringstream.cpp:162:7:162:9 | ss2 | stringstream.cpp:162:16:162:17 | ref arg b6 | TAINT | | stringstream.cpp:162:16:162:17 | ref arg b6 | stringstream.cpp:168:7:168:8 | b6 | | | stringstream.cpp:163:7:163:9 | ref arg ss1 | stringstream.cpp:165:7:165:9 | ss1 | | | stringstream.cpp:163:7:163:9 | ref arg ss1 | stringstream.cpp:174:12:174:14 | ss1 | | | stringstream.cpp:163:7:163:9 | ref arg ss1 | stringstream.cpp:176:12:176:14 | ss1 | | | stringstream.cpp:163:7:163:9 | ref arg ss1 | stringstream.cpp:178:7:178:9 | ss1 | | +| stringstream.cpp:163:7:163:9 | ss1 | stringstream.cpp:163:20:163:21 | ref arg b7 | TAINT | | stringstream.cpp:163:20:163:21 | ref arg b7 | stringstream.cpp:169:7:169:8 | b7 | | | stringstream.cpp:164:7:164:9 | ref arg ss2 | stringstream.cpp:166:7:166:9 | ss2 | | | stringstream.cpp:164:7:164:9 | ref arg ss2 | stringstream.cpp:175:12:175:14 | ss2 | | | stringstream.cpp:164:7:164:9 | ref arg ss2 | stringstream.cpp:177:12:177:14 | ss2 | | | stringstream.cpp:164:7:164:9 | ref arg ss2 | stringstream.cpp:179:7:179:9 | ss2 | | +| stringstream.cpp:164:7:164:9 | ss2 | stringstream.cpp:164:20:164:21 | ref arg b8 | TAINT | | stringstream.cpp:164:20:164:21 | ref arg b8 | stringstream.cpp:170:7:170:8 | b8 | | | stringstream.cpp:165:7:165:9 | ref arg ss1 | stringstream.cpp:174:12:174:14 | ss1 | | | stringstream.cpp:165:7:165:9 | ref arg ss1 | stringstream.cpp:176:12:176:14 | ss1 | | | stringstream.cpp:165:7:165:9 | ref arg ss1 | stringstream.cpp:178:7:178:9 | ss1 | | +| stringstream.cpp:165:7:165:9 | ss1 | stringstream.cpp:165:11:165:13 | call to get | | +| stringstream.cpp:165:7:165:9 | ss1 | stringstream.cpp:165:15:165:16 | ref arg b9 | TAINT | | stringstream.cpp:165:15:165:16 | ref arg b9 | stringstream.cpp:171:7:171:8 | b9 | | | stringstream.cpp:166:7:166:9 | ref arg ss2 | stringstream.cpp:175:12:175:14 | ss2 | | | stringstream.cpp:166:7:166:9 | ref arg ss2 | stringstream.cpp:177:12:177:14 | ss2 | | | stringstream.cpp:166:7:166:9 | ref arg ss2 | stringstream.cpp:179:7:179:9 | ss2 | | +| stringstream.cpp:166:7:166:9 | ss2 | stringstream.cpp:166:11:166:13 | call to get | | +| stringstream.cpp:166:7:166:9 | ss2 | stringstream.cpp:166:15:166:17 | ref arg b10 | TAINT | | stringstream.cpp:166:15:166:17 | ref arg b10 | stringstream.cpp:172:7:172:9 | b10 | | | stringstream.cpp:167:7:167:8 | b5 | stringstream.cpp:167:7:167:8 | call to basic_string | TAINT | | stringstream.cpp:168:7:168:8 | b6 | stringstream.cpp:168:7:168:8 | call to basic_string | TAINT | @@ -1824,22 +1834,30 @@ | stringstream.cpp:174:7:174:8 | c1 | stringstream.cpp:174:7:174:20 | ... = ... | | | stringstream.cpp:174:12:174:14 | ref arg ss1 | stringstream.cpp:176:12:176:14 | ss1 | | | stringstream.cpp:174:12:174:14 | ref arg ss1 | stringstream.cpp:178:7:178:9 | ss1 | | +| stringstream.cpp:174:12:174:14 | ss1 | stringstream.cpp:174:16:174:18 | call to get | TAINT | | stringstream.cpp:174:16:174:18 | call to get | stringstream.cpp:174:7:174:20 | ... = ... | | | stringstream.cpp:174:16:174:18 | call to get | stringstream.cpp:180:7:180:8 | c1 | | | stringstream.cpp:175:7:175:8 | c2 | stringstream.cpp:175:7:175:20 | ... = ... | | | stringstream.cpp:175:12:175:14 | ref arg ss2 | stringstream.cpp:177:12:177:14 | ss2 | | | stringstream.cpp:175:12:175:14 | ref arg ss2 | stringstream.cpp:179:7:179:9 | ss2 | | +| stringstream.cpp:175:12:175:14 | ss2 | stringstream.cpp:175:16:175:18 | call to get | TAINT | | stringstream.cpp:175:16:175:18 | call to get | stringstream.cpp:175:7:175:20 | ... = ... | | | stringstream.cpp:175:16:175:18 | call to get | stringstream.cpp:181:7:181:8 | c2 | | | stringstream.cpp:176:7:176:8 | c3 | stringstream.cpp:176:7:176:21 | ... = ... | | | stringstream.cpp:176:12:176:14 | ref arg ss1 | stringstream.cpp:178:7:178:9 | ss1 | | +| stringstream.cpp:176:12:176:14 | ss1 | stringstream.cpp:176:16:176:19 | call to peek | TAINT | | stringstream.cpp:176:16:176:19 | call to peek | stringstream.cpp:176:7:176:21 | ... = ... | | | stringstream.cpp:176:16:176:19 | call to peek | stringstream.cpp:182:7:182:8 | c3 | | | stringstream.cpp:177:7:177:8 | c4 | stringstream.cpp:177:7:177:21 | ... = ... | | | stringstream.cpp:177:12:177:14 | ref arg ss2 | stringstream.cpp:179:7:179:9 | ss2 | | +| stringstream.cpp:177:12:177:14 | ss2 | stringstream.cpp:177:16:177:19 | call to peek | TAINT | | stringstream.cpp:177:16:177:19 | call to peek | stringstream.cpp:177:7:177:21 | ... = ... | | | stringstream.cpp:177:16:177:19 | call to peek | stringstream.cpp:183:7:183:8 | c4 | | +| stringstream.cpp:178:7:178:9 | ss1 | stringstream.cpp:178:11:178:13 | call to get | | +| stringstream.cpp:178:7:178:9 | ss1 | stringstream.cpp:178:15:178:16 | ref arg c5 | TAINT | | stringstream.cpp:178:15:178:16 | ref arg c5 | stringstream.cpp:184:7:184:8 | c5 | | +| stringstream.cpp:179:7:179:9 | ss2 | stringstream.cpp:179:11:179:13 | call to get | | +| stringstream.cpp:179:7:179:9 | ss2 | stringstream.cpp:179:15:179:16 | ref arg c6 | TAINT | | stringstream.cpp:179:15:179:16 | ref arg c6 | stringstream.cpp:185:7:185:8 | c6 | | | stringstream.cpp:190:20:190:21 | call to basic_stringstream | stringstream.cpp:192:7:192:8 | ss | | | stringstream.cpp:190:20:190:21 | call to basic_stringstream | stringstream.cpp:193:7:193:8 | ss | | @@ -1859,12 +1877,15 @@ | stringstream.cpp:193:7:193:8 | ref arg ss | stringstream.cpp:195:7:195:8 | ss | | | stringstream.cpp:193:7:193:8 | ref arg ss | stringstream.cpp:196:7:196:8 | ss | | | stringstream.cpp:193:7:193:8 | ref arg ss | stringstream.cpp:197:7:197:8 | ss | | +| stringstream.cpp:193:7:193:8 | ss | stringstream.cpp:193:10:193:12 | call to get | TAINT | | stringstream.cpp:194:7:194:8 | ref arg ss | stringstream.cpp:195:7:195:8 | ss | | | stringstream.cpp:194:7:194:8 | ref arg ss | stringstream.cpp:196:7:196:8 | ss | | | stringstream.cpp:194:7:194:8 | ref arg ss | stringstream.cpp:197:7:197:8 | ss | | | stringstream.cpp:195:7:195:8 | ref arg ss | stringstream.cpp:196:7:196:8 | ss | | | stringstream.cpp:195:7:195:8 | ref arg ss | stringstream.cpp:197:7:197:8 | ss | | +| stringstream.cpp:195:7:195:8 | ss | stringstream.cpp:195:10:195:12 | call to get | TAINT | | stringstream.cpp:196:7:196:8 | ref arg ss | stringstream.cpp:197:7:197:8 | ss | | +| stringstream.cpp:197:7:197:8 | ss | stringstream.cpp:197:10:197:12 | call to get | TAINT | | structlikeclass.cpp:5:7:5:7 | Unknown literal | structlikeclass.cpp:5:7:5:7 | constructor init of field v | TAINT | | structlikeclass.cpp:5:7:5:7 | Unknown literal | structlikeclass.cpp:5:7:5:7 | constructor init of field v | TAINT | | structlikeclass.cpp:5:7:5:7 | this | structlikeclass.cpp:5:7:5:7 | constructor init of field v [pre-this] | | diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp index 6f5102a8270..394d4f70a2f 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp @@ -159,30 +159,30 @@ void test_stringstream_in() sink(b4); // tainted sink(ss1.read(b5, 100)); - sink(ss2.read(b6, 100)); // tainted [NOT DETECTED] + sink(ss2.read(b6, 100)); // tainted sink(ss1.readsome(b7, 100)); sink(ss2.readsome(b8, 100)); // (returns a length, not significantly tainted) sink(ss1.get(b9, 100)); - sink(ss2.get(b10, 100)); + sink(ss2.get(b10, 100)); // tainted sink(b5); - sink(b6); // tainted [NOT DETECTED] + sink(b6); // tainted sink(b7); - sink(b8); // tainted [NOT DETECTED] + sink(b8); // tainted sink(b9); - sink(b10); // tainted [NOT DETECTED] + sink(b10); // tainted sink(c1 = ss1.get()); - sink(c2 = ss2.get()); // tainted [NOT DETECTED] + sink(c2 = ss2.get()); // tainted sink(c3 = ss1.peek()); - sink(c4 = ss2.peek()); // tainted [NOT DETECTED] + sink(c4 = ss2.peek()); // tainted sink(ss1.get(c5)); - sink(ss2.get(c6)); // tainted [NOT DETECTED] + sink(ss2.get(c6)); // tainted sink(c1); - sink(c2); // tainted [NOT DETECTED] + sink(c2); // tainted sink(c3); - sink(c4); // tainted [NOT DETECTED] + sink(c4); // tainted sink(c5); - sink(c6); // tainted [NOT DETECTED] + sink(c6); // tainted } void test_stringstream_putback() 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 8ed02e04295..1590d5f724f 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected @@ -209,6 +209,17 @@ | stringstream.cpp:157:7:157:8 | call to basic_string | stringstream.cpp:143:14:143:19 | call to source | | stringstream.cpp:158:7:158:8 | call to basic_string | stringstream.cpp:143:14:143:19 | call to source | | stringstream.cpp:159:7:159:8 | call to basic_string | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:162:11:162:14 | call to read | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:166:11:166:13 | call to get | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:168:7:168:8 | call to basic_string | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:170:7:170:8 | call to basic_string | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:172:7:172:9 | call to basic_string | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:175:7:175:20 | ... = ... | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:177:7:177:21 | ... = ... | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:179:11:179:13 | call to get | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:181:7:181:8 | c2 | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:183:7:183:8 | c4 | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:185:7:185:8 | c6 | stringstream.cpp:143:14:143:19 | call to source | | structlikeclass.cpp:35:8:35:9 | s1 | structlikeclass.cpp:29:22:29:27 | call to source | | structlikeclass.cpp:36:8:36:9 | s2 | structlikeclass.cpp:30:24:30:29 | call to source | | structlikeclass.cpp:37:8:37:9 | s3 | structlikeclass.cpp:29:22:29:27 | 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 e8bc4f0ad4a..f7356120998 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 @@ -211,6 +211,17 @@ | stringstream.cpp:157:7:157:8 | stringstream.cpp:143:14:143:21 | IR only | | stringstream.cpp:158:7:158:8 | stringstream.cpp:143:14:143:21 | IR only | | stringstream.cpp:159:7:159:8 | stringstream.cpp:143:14:143:19 | AST only | +| stringstream.cpp:162:11:162:14 | stringstream.cpp:143:14:143:19 | AST only | +| stringstream.cpp:166:11:166:13 | stringstream.cpp:143:14:143:19 | AST only | +| stringstream.cpp:168:7:168:8 | stringstream.cpp:143:14:143:19 | AST only | +| stringstream.cpp:170:7:170:8 | stringstream.cpp:143:14:143:19 | AST only | +| stringstream.cpp:172:7:172:9 | stringstream.cpp:143:14:143:19 | AST only | +| stringstream.cpp:175:7:175:20 | stringstream.cpp:143:14:143:19 | AST only | +| stringstream.cpp:177:7:177:21 | stringstream.cpp:143:14:143:19 | AST only | +| stringstream.cpp:179:11:179:13 | stringstream.cpp:143:14:143:19 | AST only | +| stringstream.cpp:181:7:181:8 | stringstream.cpp:143:14:143:19 | AST only | +| stringstream.cpp:183:7:183:8 | stringstream.cpp:143:14:143:19 | AST only | +| stringstream.cpp:185:7:185:8 | stringstream.cpp:143:14:143:19 | AST only | | swap1.cpp:78:12:78:16 | swap1.cpp:69:23:69:23 | AST only | | swap1.cpp:87:13:87:17 | swap1.cpp:82:16:82:21 | AST only | | swap1.cpp:88:13:88:17 | swap1.cpp:81:27:81:28 | AST only | From eb7bd6e1765d33595b663979b048a16bd96fe37f Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Mon, 14 Sep 2020 11:04:45 +0100 Subject: [PATCH 3/7] C++: Flow through putback. --- .../semmle/code/cpp/models/implementations/StdString.qll | 7 +++++-- .../library-tests/dataflow/taint-tests/localTaint.expected | 6 ++++++ .../library-tests/dataflow/taint-tests/stringstream.cpp | 4 ++-- .../test/library-tests/dataflow/taint-tests/taint.expected | 2 ++ .../library-tests/dataflow/taint-tests/test_diff.expected | 2 ++ 5 files changed, 17 insertions(+), 4 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll index 5797a76acc6..869d77fec57 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll @@ -409,10 +409,13 @@ class StdBasicOStream extends TemplateClass { /** * The `std::ostream` functions `operator<<` (defined as a member function), - * `put` and `write`. + * `put` and `write` and `std::istream::putback`. */ class StdOStreamOut extends DataFlowFunction, TaintFunction { - StdOStreamOut() { this.hasQualifiedName("std", "basic_ostream", ["operator<<", "put", "write"]) } + StdOStreamOut() { + this.hasQualifiedName("std", "basic_ostream", ["operator<<", "put", "write"]) or + this.hasQualifiedName("std", "basic_istream", "putback") + } override predicate hasDataFlow(FunctionInput input, FunctionOutput output) { // flow from qualifier to return value 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 a3ed313206c..6751b28d544 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected @@ -1881,10 +1881,16 @@ | stringstream.cpp:194:7:194:8 | ref arg ss | stringstream.cpp:195:7:195:8 | ss | | | stringstream.cpp:194:7:194:8 | ref arg ss | stringstream.cpp:196:7:196:8 | ss | | | stringstream.cpp:194:7:194:8 | ref arg ss | stringstream.cpp:197:7:197:8 | ss | | +| stringstream.cpp:194:7:194:8 | ss | stringstream.cpp:194:10:194:16 | call to putback | | +| stringstream.cpp:194:18:194:20 | 98 | stringstream.cpp:194:7:194:8 | ref arg ss | TAINT | +| stringstream.cpp:194:18:194:20 | 98 | stringstream.cpp:194:10:194:16 | call to putback | TAINT | | stringstream.cpp:195:7:195:8 | ref arg ss | stringstream.cpp:196:7:196:8 | ss | | | stringstream.cpp:195:7:195:8 | ref arg ss | stringstream.cpp:197:7:197:8 | ss | | | stringstream.cpp:195:7:195:8 | ss | stringstream.cpp:195:10:195:12 | call to get | TAINT | | stringstream.cpp:196:7:196:8 | ref arg ss | stringstream.cpp:197:7:197:8 | ss | | +| stringstream.cpp:196:7:196:8 | ss | stringstream.cpp:196:10:196:16 | call to putback | | +| stringstream.cpp:196:18:196:32 | call to source | stringstream.cpp:196:7:196:8 | ref arg ss | TAINT | +| stringstream.cpp:196:18:196:32 | call to source | stringstream.cpp:196:10:196:16 | call to putback | TAINT | | stringstream.cpp:197:7:197:8 | ss | stringstream.cpp:197:10:197:12 | call to get | TAINT | | structlikeclass.cpp:5:7:5:7 | Unknown literal | structlikeclass.cpp:5:7:5:7 | constructor init of field v | TAINT | | structlikeclass.cpp:5:7:5:7 | Unknown literal | structlikeclass.cpp:5:7:5:7 | constructor init of field v | TAINT | diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp index 394d4f70a2f..3503ef3854a 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp @@ -193,6 +193,6 @@ void test_stringstream_putback() sink(ss.get()); sink(ss.putback('b')); sink(ss.get()); - sink(ss.putback(ns_char::source())); // tainted [NOT DETECTED] - sink(ss.get()); // tainted [NOT DETECTED] + sink(ss.putback(ns_char::source())); // tainted + sink(ss.get()); // 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 1590d5f724f..6a4b3f6f2b7 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected @@ -220,6 +220,8 @@ | stringstream.cpp:181:7:181:8 | c2 | stringstream.cpp:143:14:143:19 | call to source | | stringstream.cpp:183:7:183:8 | c4 | stringstream.cpp:143:14:143:19 | call to source | | stringstream.cpp:185:7:185:8 | c6 | stringstream.cpp:143:14:143:19 | call to source | +| stringstream.cpp:196:10:196:16 | call to putback | stringstream.cpp:196:18:196:32 | call to source | +| stringstream.cpp:197:10:197:12 | call to get | stringstream.cpp:196:18:196:32 | call to source | | structlikeclass.cpp:35:8:35:9 | s1 | structlikeclass.cpp:29:22:29:27 | call to source | | structlikeclass.cpp:36:8:36:9 | s2 | structlikeclass.cpp:30:24:30:29 | call to source | | structlikeclass.cpp:37:8:37:9 | s3 | structlikeclass.cpp:29:22:29:27 | 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 f7356120998..f30ba31b463 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 @@ -222,6 +222,8 @@ | stringstream.cpp:181:7:181:8 | stringstream.cpp:143:14:143:19 | AST only | | stringstream.cpp:183:7:183:8 | stringstream.cpp:143:14:143:19 | AST only | | stringstream.cpp:185:7:185:8 | stringstream.cpp:143:14:143:19 | AST only | +| stringstream.cpp:196:10:196:16 | stringstream.cpp:196:18:196:32 | AST only | +| stringstream.cpp:197:10:197:12 | stringstream.cpp:196:18:196:32 | AST only | | swap1.cpp:78:12:78:16 | swap1.cpp:69:23:69:23 | AST only | | swap1.cpp:87:13:87:17 | swap1.cpp:82:16:82:21 | AST only | | swap1.cpp:88:13:88:17 | swap1.cpp:81:27:81:28 | AST only | From c4de071a4c2eddce353071e9506f1eaa87dbc6af Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Mon, 14 Sep 2020 11:14:06 +0100 Subject: [PATCH 4/7] C++: Flow through swap. --- .../semmle/code/cpp/models/implementations/StdString.qll | 7 +++++-- .../library-tests/dataflow/taint-tests/localTaint.expected | 4 ++++ .../library-tests/dataflow/taint-tests/stringstream.cpp | 4 ++-- .../test/library-tests/dataflow/taint-tests/taint.expected | 2 ++ .../library-tests/dataflow/taint-tests/test_diff.expected | 2 ++ 5 files changed, 15 insertions(+), 4 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll index 869d77fec57..76beb6f2528 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll @@ -256,10 +256,13 @@ class StdStringSubstr extends TaintFunction { } /** - * The standard function `std::string.swap`. + * The standard functions `std::string.swap` and `std::stringstream::swap`. */ class StdStringSwap extends TaintFunction { - StdStringSwap() { this.hasQualifiedName("std", "basic_string", "swap") } + StdStringSwap() { + this.hasQualifiedName("std", "basic_string", "swap") or + this.hasQualifiedName("std", "basic_stringstream", "swap") + } override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { // str1.swap(str2) 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 6751b28d544..6736ae4a440 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected @@ -1623,9 +1623,13 @@ | stringstream.cpp:115:24:115:32 | call to basic_stringstream | stringstream.cpp:118:2:118:4 | ss4 | | | stringstream.cpp:115:24:115:32 | call to basic_stringstream | stringstream.cpp:123:7:123:9 | ss4 | | | stringstream.cpp:117:2:117:4 | ref arg ss1 | stringstream.cpp:120:7:120:9 | ss1 | | +| stringstream.cpp:117:2:117:4 | ss1 | stringstream.cpp:117:11:117:13 | ref arg ss2 | TAINT | | stringstream.cpp:117:11:117:13 | ref arg ss2 | stringstream.cpp:121:7:121:9 | ss2 | | +| stringstream.cpp:117:11:117:13 | ss2 | stringstream.cpp:117:2:117:4 | ref arg ss1 | TAINT | | stringstream.cpp:118:2:118:4 | ref arg ss4 | stringstream.cpp:123:7:123:9 | ss4 | | +| stringstream.cpp:118:2:118:4 | ss4 | stringstream.cpp:118:11:118:13 | ref arg ss3 | TAINT | | stringstream.cpp:118:11:118:13 | ref arg ss3 | stringstream.cpp:122:7:122:9 | ss3 | | +| stringstream.cpp:118:11:118:13 | ss3 | stringstream.cpp:118:2:118:4 | ref arg ss4 | TAINT | | stringstream.cpp:128:20:128:22 | call to basic_stringstream | stringstream.cpp:142:7:142:9 | ss1 | | | stringstream.cpp:128:20:128:22 | call to basic_stringstream | stringstream.cpp:145:7:145:9 | ss1 | | | stringstream.cpp:128:20:128:22 | call to basic_stringstream | stringstream.cpp:153:7:153:9 | ss1 | | diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp index 3503ef3854a..f2563ba9277 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp @@ -117,9 +117,9 @@ void test_stringstream_swap() ss1.swap(ss2); ss4.swap(ss3); - sink(ss1); // tainted [NOT DETECTED] + sink(ss1); // tainted sink(ss2); // [FALSE POSITIVE] - sink(ss3); // tainted [NOT DETECTED] + sink(ss3); // tainted sink(ss4); // [FALSE POSITIVE] } 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 6a4b3f6f2b7..25e0b950352 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected @@ -196,7 +196,9 @@ | stringstream.cpp:103:7:103:9 | ss2 | stringstream.cpp:91:19:91:24 | call to source | | stringstream.cpp:105:7:105:9 | ss4 | stringstream.cpp:95:44:95:49 | call to source | | stringstream.cpp:107:7:107:9 | ss6 | stringstream.cpp:100:31:100:36 | call to source | +| stringstream.cpp:120:7:120:9 | ss1 | stringstream.cpp:113:24:113:29 | call to source | | stringstream.cpp:121:7:121:9 | ss2 | stringstream.cpp:113:24:113:29 | call to source | +| stringstream.cpp:122:7:122:9 | ss3 | stringstream.cpp:115:24:115:29 | call to source | | stringstream.cpp:123:7:123:9 | ss4 | stringstream.cpp:115:24:115:29 | call to source | | stringstream.cpp:143:11:143:11 | call to operator<< | stringstream.cpp:143:14:143:19 | call to source | | stringstream.cpp:146:11:146:11 | call to operator>> | stringstream.cpp:143:14:143:19 | 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 f30ba31b463..f1b10f11022 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 @@ -196,7 +196,9 @@ | stringstream.cpp:103:7:103:9 | stringstream.cpp:91:19:91:24 | AST only | | stringstream.cpp:105:7:105:9 | stringstream.cpp:95:44:95:49 | AST only | | stringstream.cpp:107:7:107:9 | stringstream.cpp:100:31:100:36 | AST only | +| stringstream.cpp:120:7:120:9 | stringstream.cpp:113:24:113:29 | AST only | | stringstream.cpp:121:7:121:9 | stringstream.cpp:113:24:113:29 | AST only | +| stringstream.cpp:122:7:122:9 | stringstream.cpp:115:24:115:29 | AST only | | stringstream.cpp:123:7:123:9 | stringstream.cpp:115:24:115:29 | AST only | | stringstream.cpp:143:11:143:11 | stringstream.cpp:143:14:143:21 | IR only | | stringstream.cpp:143:11:143:22 | stringstream.cpp:143:14:143:19 | IR only | From eedbe839b5626daf213a3c90bb33ca52048acf98 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Mon, 14 Sep 2020 11:15:46 +0100 Subject: [PATCH 5/7] C++: Update change note. --- change-notes/1.26/analysis-cpp.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/change-notes/1.26/analysis-cpp.md b/change-notes/1.26/analysis-cpp.md index 82d74e5d6b4..66386bbce5d 100644 --- a/change-notes/1.26/analysis-cpp.md +++ b/change-notes/1.26/analysis-cpp.md @@ -23,7 +23,7 @@ The following changes in version 1.26 affect C/C++ analysis in all applications. * The QL class `Block`, denoting the `{ ... }` statement, is renamed to `BlockStmt`. * The models library now models many taint flows through `std::array`, `std::vector`, `std::deque`, `std::list` and `std::forward_list`. * The models library now models many more taint flows through `std::string`. -* The models library now models some taint flows through `std::ostream`. +* The models library now models many taint flows through `std::istream` and `std::ostream`. * The models library now models some taint flows through `std::shared_ptr`, `std::unique_ptr`, `std::make_shared` and `std::make_unique`. * The `SimpleRangeAnalysis` library now supports multiplications of the form `e1 * e2` and `x *= e2` when `e1` and `e2` are unsigned or constant. From f1a9547b380a7061e8200bfe44aaef2455d994c9 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 16 Sep 2020 16:44:39 +0100 Subject: [PATCH 6/7] C++: Split off putback. --- .../cpp/models/implementations/StdString.qll | 40 ++++++++++++++++--- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll index 76beb6f2528..d30d45f1e0d 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll @@ -403,6 +403,39 @@ class StdIStreamReadSome extends TaintFunction { } } +/** + * The `std::istream` function `putback`. + */ +class StdIStreamPutBack extends DataFlowFunction, TaintFunction { + StdIStreamPutBack() { this.hasQualifiedName("std", "basic_istream", "putback") } + + override predicate hasDataFlow(FunctionInput input, FunctionOutput output) { + // flow from qualifier to return value + input.isQualifierAddress() and + output.isReturnValue() + } + + override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { + // flow from first parameter (value or pointer) to qualifier + input.isParameter(0) and + output.isQualifierObject() + or + input.isParameterDeref(0) and + output.isQualifierObject() + or + // flow from first parameter (value or pointer) to return value + input.isParameter(0) and + output.isReturnValueDeref() + or + input.isParameterDeref(0) and + output.isReturnValueDeref() + or + // reverse flow from returned reference to the qualifier + input.isReturnValueDeref() and + output.isQualifierObject() + } +} + /** * The `std::basic_ostream` template class. */ @@ -412,13 +445,10 @@ class StdBasicOStream extends TemplateClass { /** * The `std::ostream` functions `operator<<` (defined as a member function), - * `put` and `write` and `std::istream::putback`. + * `put` and `write`. */ class StdOStreamOut extends DataFlowFunction, TaintFunction { - StdOStreamOut() { - this.hasQualifiedName("std", "basic_ostream", ["operator<<", "put", "write"]) or - this.hasQualifiedName("std", "basic_istream", "putback") - } + StdOStreamOut() { this.hasQualifiedName("std", "basic_ostream", ["operator<<", "put", "write"]) } override predicate hasDataFlow(FunctionInput input, FunctionOutput output) { // flow from qualifier to return value From c17ae3ad6c01fd1580e8ae3636438741823d3373 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 17 Sep 2020 11:34:10 +0100 Subject: [PATCH 7/7] C++: Correct dataflow for return (*this). --- .../code/cpp/models/implementations/StdString.qll | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll index d30d45f1e0d..ddd813a62ae 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll @@ -305,8 +305,8 @@ class StdIStreamIn extends DataFlowFunction, TaintFunction { StdIStreamIn() { this.hasQualifiedName("std", "basic_istream", "operator>>") } override predicate hasDataFlow(FunctionInput input, FunctionOutput output) { - // flow from qualifier to return value - input.isQualifierObject() and + // returns reference to `*this` + input.isQualifierAddress() and output.isReturnValue() } @@ -374,8 +374,8 @@ class StdIStreamRead extends DataFlowFunction, TaintFunction { } override predicate hasDataFlow(FunctionInput input, FunctionOutput output) { - // flow from qualifier to return value - input.isQualifierObject() and + // returns reference to `*this` + input.isQualifierAddress() and output.isReturnValue() } @@ -410,7 +410,7 @@ class StdIStreamPutBack extends DataFlowFunction, TaintFunction { StdIStreamPutBack() { this.hasQualifiedName("std", "basic_istream", "putback") } override predicate hasDataFlow(FunctionInput input, FunctionOutput output) { - // flow from qualifier to return value + // returns reference to `*this` input.isQualifierAddress() and output.isReturnValue() } @@ -451,7 +451,7 @@ class StdOStreamOut extends DataFlowFunction, TaintFunction { StdOStreamOut() { this.hasQualifiedName("std", "basic_ostream", ["operator<<", "put", "write"]) } override predicate hasDataFlow(FunctionInput input, FunctionOutput output) { - // flow from qualifier to return value + // returns reference to `*this` input.isQualifierAddress() and output.isReturnValue() }