Skip to content

Commit ed06604

Browse files
authored
Merge pull request github#4045 from geoffw0/plus
C++: Model more of std::string in models.
2 parents de87f8f + 498b350 commit ed06604

File tree

7 files changed

+465
-243
lines changed

7 files changed

+465
-243
lines changed

change-notes/1.26/analysis-cpp.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,6 @@ The following changes in version 1.26 affect C/C++ analysis in all applications.
1818

1919
## Changes to libraries
2020

21+
* The models library now models more taint flows through `std::string`.
2122
* The `SimpleRangeAnalysis` library now supports multiplications of the form
2223
`e1 * e2` when `e1` and `e2` are unsigned.

cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
import semmle.code.cpp.models.interfaces.Taint
22

3+
/**
4+
* The `std::basic_string` template class.
5+
*/
6+
class StdBasicString extends TemplateClass {
7+
StdBasicString() { this.hasQualifiedName("std", "basic_string") }
8+
}
9+
310
/**
411
* The standard function `std::string.c_str`.
512
*/
@@ -12,3 +19,51 @@ class StdStringCStr extends TaintFunction {
1219
output.isReturnValue()
1320
}
1421
}
22+
23+
/**
24+
* The `std::string` function `operator+`.
25+
*/
26+
class StdStringPlus extends TaintFunction {
27+
StdStringPlus() {
28+
this.hasQualifiedName("std", "operator+") and
29+
this.getUnspecifiedType() = any(StdBasicString s).getAnInstantiation()
30+
}
31+
32+
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
33+
// flow from parameters to return value
34+
(
35+
input.isParameterDeref(0) or
36+
input.isParameterDeref(1)
37+
) and
38+
output.isReturnValue()
39+
}
40+
}
41+
42+
/**
43+
* The `std::string` functions `operator+=` and `append`.
44+
*/
45+
class StdStringAppend extends TaintFunction {
46+
StdStringAppend() {
47+
this.hasQualifiedName("std", "basic_string", "operator+=") or
48+
this.hasQualifiedName("std", "basic_string", "append")
49+
}
50+
51+
/**
52+
* Gets the index of a parameter to this function that is a string (or
53+
* character).
54+
*/
55+
int getAStringParameter() {
56+
getParameter(result).getType() instanceof PointerType or
57+
getParameter(result).getType() instanceof ReferenceType or
58+
getParameter(result).getType() = getDeclaringType().getTemplateArgument(0) // i.e. `std::basic_string::CharT`
59+
}
60+
61+
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
62+
// flow from parameter to string itself (qualifier) and return value
63+
input.isParameterDeref(getAStringParameter()) and
64+
(
65+
output.isQualifierObject() or
66+
output.isReturnValueDeref()
67+
)
68+
}
69+
}

cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected

Lines changed: 277 additions & 198 deletions
Large diffs are not rendered by default.

cpp/ql/test/library-tests/dataflow/taint-tests/stl.cpp

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,14 @@ namespace std
3030
template <class T> class allocator {
3131
public:
3232
allocator() throw();
33+
typedef size_t size_type;
3334
};
3435

3536
template<class charT, class traits = char_traits<charT>, class Allocator = allocator<charT> >
3637
class basic_string {
3738
public:
39+
typedef typename Allocator::size_type size_type;
40+
3841
explicit basic_string(const Allocator& a = Allocator());
3942
basic_string(const charT* s, const Allocator& a = Allocator());
4043

@@ -49,8 +52,17 @@ namespace std
4952
const_iterator end() const;
5053
const_iterator cbegin() const;
5154
const_iterator cend() const;
55+
56+
template<class T> basic_string& operator+=(const T& t);
57+
basic_string& operator+=(const charT* s);
58+
basic_string& append(const basic_string& str);
59+
basic_string& append(const charT* s);
60+
basic_string& append(size_type n, charT c);
5261
};
5362

63+
template<class charT, class traits, class Allocator> basic_string<charT, traits, Allocator> operator+(const basic_string<charT, traits, Allocator>& lhs, const basic_string<charT, traits, Allocator>& rhs);
64+
template<class charT, class traits, class Allocator> basic_string<charT, traits, Allocator> operator+(const basic_string<charT, traits, Allocator>& lhs, const charT* rhs);
65+
5466
typedef basic_string<char> string;
5567

5668
template <class charT, class traits = char_traits<charT> >
@@ -308,3 +320,58 @@ void test_range_based_for_loop_vector(int source1) {
308320
sink(x); // tainted [NOT DETECTED by IR]
309321
}
310322
}
323+
324+
namespace ns_char
325+
{
326+
char source();
327+
}
328+
329+
void test_string_append() {
330+
{
331+
std::string s1("hello");
332+
std::string s2(source());
333+
334+
sink(s1 + s1);
335+
sink(s1 + s2); // tainted
336+
sink(s2 + s1); // tainted
337+
sink(s2 + s2); // tainted
338+
339+
sink(s1 + " world");
340+
sink(s1 + source()); // tainted
341+
}
342+
343+
{
344+
std::string s3("abc");
345+
std::string s4(source());
346+
std::string s5, s6, s7, s8, s9;
347+
348+
s5 = s3 + s4;
349+
sink(s5); // tainted
350+
351+
s6 = s3;
352+
s6 += s4;
353+
sink(s6); // tainted
354+
355+
s7 = s3;
356+
s7 += source();
357+
s7 += " ";
358+
sink(s7); // tainted
359+
360+
s8 = s3;
361+
s8.append(s4);
362+
sink(s8); // tainted
363+
364+
s9 = s3;
365+
s9.append(source());
366+
s9.append(" ");
367+
sink(s9); // tainted
368+
}
369+
370+
{
371+
std::string s10("abc");
372+
char c = ns_char::source();
373+
374+
s10.append(1, c);
375+
sink(s10); // tainted
376+
}
377+
}

cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -32,27 +32,37 @@
3232
| movableclass.cpp:55:8:55:9 | s2 | movableclass.cpp:52:23:52:28 | call to source |
3333
| movableclass.cpp:64:8:64:9 | s2 | movableclass.cpp:23:55:23:60 | call to source |
3434
| movableclass.cpp:65:11:65:11 | call to operator= | movableclass.cpp:65:13:65:18 | call to source |
35-
| stl.cpp:101:7:101:7 | a | stl.cpp:97:12:97:17 | call to source |
36-
| stl.cpp:103:7:103:7 | c | stl.cpp:99:16:99:21 | call to source |
37-
| stl.cpp:105:9:105:13 | call to c_str | stl.cpp:99:16:99:21 | call to source |
38-
| stl.cpp:155:13:155:17 | call to c_str | stl.cpp:147:10:147:15 | call to source |
39-
| stl.cpp:159:13:159:17 | call to c_str | stl.cpp:147:10:147:15 | call to source |
40-
| stl.cpp:162:13:162:17 | call to c_str | stl.cpp:147:10:147:15 | call to source |
41-
| stl.cpp:172:7:172:8 | cs | stl.cpp:167:19:167:24 | call to source |
42-
| stl.cpp:173:7:173:8 | ss | stl.cpp:167:19:167:24 | call to source |
43-
| stl.cpp:186:7:186:8 | cs | stl.cpp:178:19:178:24 | call to source |
44-
| stl.cpp:187:7:187:8 | ss | stl.cpp:178:19:178:24 | call to source |
45-
| stl.cpp:209:8:209:9 | s1 | stl.cpp:204:18:204:23 | call to source |
46-
| stl.cpp:210:8:210:9 | s2 | stl.cpp:205:20:205:25 | call to source |
47-
| stl.cpp:211:8:211:9 | s3 | stl.cpp:207:8:207:13 | call to source |
48-
| stl.cpp:230:8:230:9 | s1 | stl.cpp:226:32:226:37 | call to source |
49-
| stl.cpp:231:8:231:9 | s2 | stl.cpp:228:20:228:25 | call to source |
50-
| stl.cpp:240:8:240:8 | c | stl.cpp:238:16:238:21 | call to source |
51-
| stl.cpp:248:8:248:8 | c | stl.cpp:238:16:238:21 | call to source |
52-
| stl.cpp:253:8:253:8 | c | stl.cpp:251:28:251:33 | call to source |
53-
| stl.cpp:295:8:295:8 | x | stl.cpp:288:43:288:49 | source1 |
54-
| stl.cpp:303:8:303:8 | x | stl.cpp:288:43:288:49 | source1 |
55-
| stl.cpp:308:8:308:8 | x | stl.cpp:288:43:288:49 | source1 |
35+
| stl.cpp:113:7:113:7 | a | stl.cpp:109:12:109:17 | call to source |
36+
| stl.cpp:115:7:115:7 | c | stl.cpp:111:16:111:21 | call to source |
37+
| stl.cpp:117:9:117:13 | call to c_str | stl.cpp:111:16:111:21 | call to source |
38+
| stl.cpp:167:13:167:17 | call to c_str | stl.cpp:159:10:159:15 | call to source |
39+
| stl.cpp:171:13:171:17 | call to c_str | stl.cpp:159:10:159:15 | call to source |
40+
| stl.cpp:174:13:174:17 | call to c_str | stl.cpp:159:10:159:15 | call to source |
41+
| stl.cpp:184:7:184:8 | cs | stl.cpp:179:19:179:24 | call to source |
42+
| stl.cpp:185:7:185:8 | ss | stl.cpp:179:19:179:24 | call to source |
43+
| stl.cpp:198:7:198:8 | cs | stl.cpp:190:19:190:24 | call to source |
44+
| stl.cpp:199:7:199:8 | ss | stl.cpp:190:19:190:24 | call to source |
45+
| stl.cpp:221:8:221:9 | s1 | stl.cpp:216:18:216:23 | call to source |
46+
| stl.cpp:222:8:222:9 | s2 | stl.cpp:217:20:217:25 | call to source |
47+
| stl.cpp:223:8:223:9 | s3 | stl.cpp:219:8:219:13 | call to source |
48+
| stl.cpp:242:8:242:9 | s1 | stl.cpp:238:32:238:37 | call to source |
49+
| stl.cpp:243:8:243:9 | s2 | stl.cpp:240:20:240:25 | call to source |
50+
| stl.cpp:252:8:252:8 | c | stl.cpp:250:16:250:21 | call to source |
51+
| stl.cpp:260:8:260:8 | c | stl.cpp:250:16:250:21 | call to source |
52+
| stl.cpp:265:8:265:8 | c | stl.cpp:263:28:263:33 | call to source |
53+
| stl.cpp:307:8:307:8 | x | stl.cpp:300:43:300:49 | source1 |
54+
| stl.cpp:315:8:315:8 | x | stl.cpp:300:43:300:49 | source1 |
55+
| stl.cpp:320:8:320:8 | x | stl.cpp:300:43:300:49 | source1 |
56+
| stl.cpp:335:11:335:11 | call to operator+ | stl.cpp:332:18:332:23 | call to source |
57+
| stl.cpp:336:11:336:11 | call to operator+ | stl.cpp:332:18:332:23 | call to source |
58+
| stl.cpp:337:11:337:11 | call to operator+ | stl.cpp:332:18:332:23 | call to source |
59+
| stl.cpp:340:11:340:11 | call to operator+ | stl.cpp:340:13:340:18 | call to source |
60+
| stl.cpp:349:8:349:9 | s5 | stl.cpp:345:18:345:23 | call to source |
61+
| stl.cpp:353:8:353:9 | s6 | stl.cpp:345:18:345:23 | call to source |
62+
| stl.cpp:358:8:358:9 | s7 | stl.cpp:356:9:356:14 | call to source |
63+
| stl.cpp:362:8:362:9 | s8 | stl.cpp:345:18:345:23 | call to source |
64+
| stl.cpp:367:8:367:9 | s9 | stl.cpp:365:13:365:18 | call to source |
65+
| stl.cpp:375:8:375:10 | s10 | stl.cpp:372:12:372:26 | call to source |
5666
| structlikeclass.cpp:35:8:35:9 | s1 | structlikeclass.cpp:29:22:29:27 | call to source |
5767
| structlikeclass.cpp:36:8:36:9 | s2 | structlikeclass.cpp:30:24:30:29 | call to source |
5868
| structlikeclass.cpp:37:8:37:9 | s3 | structlikeclass.cpp:29:22:29:27 | call to source |

cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -30,26 +30,36 @@
3030
| movableclass.cpp:55:8:55:9 | movableclass.cpp:52:23:52:28 | AST only |
3131
| movableclass.cpp:64:8:64:9 | movableclass.cpp:23:55:23:60 | AST only |
3232
| movableclass.cpp:65:11:65:11 | movableclass.cpp:65:13:65:18 | AST only |
33-
| stl.cpp:103:7:103:7 | stl.cpp:99:16:99:21 | AST only |
34-
| stl.cpp:105:9:105:13 | stl.cpp:99:16:99:21 | AST only |
35-
| stl.cpp:155:13:155:17 | stl.cpp:147:10:147:15 | AST only |
36-
| stl.cpp:159:13:159:17 | stl.cpp:147:10:147:15 | AST only |
37-
| stl.cpp:162:13:162:17 | stl.cpp:147:10:147:15 | AST only |
38-
| stl.cpp:172:7:172:8 | stl.cpp:167:19:167:26 | IR only |
39-
| stl.cpp:173:7:173:8 | stl.cpp:167:19:167:24 | AST only |
40-
| stl.cpp:186:7:186:8 | stl.cpp:178:19:178:24 | AST only |
41-
| stl.cpp:187:7:187:8 | stl.cpp:178:19:178:24 | AST only |
42-
| stl.cpp:209:8:209:9 | stl.cpp:204:18:204:23 | AST only |
43-
| stl.cpp:210:8:210:9 | stl.cpp:205:20:205:25 | AST only |
44-
| stl.cpp:211:8:211:9 | stl.cpp:207:8:207:13 | AST only |
45-
| stl.cpp:230:8:230:9 | stl.cpp:226:32:226:37 | AST only |
46-
| stl.cpp:231:8:231:9 | stl.cpp:228:20:228:25 | AST only |
47-
| stl.cpp:240:8:240:8 | stl.cpp:238:16:238:21 | AST only |
48-
| stl.cpp:248:8:248:8 | stl.cpp:238:16:238:21 | AST only |
49-
| stl.cpp:253:8:253:8 | stl.cpp:251:28:251:33 | AST only |
50-
| stl.cpp:295:8:295:8 | stl.cpp:288:43:288:49 | AST only |
51-
| stl.cpp:303:8:303:8 | stl.cpp:288:43:288:49 | AST only |
52-
| stl.cpp:308:8:308:8 | stl.cpp:288:43:288:49 | AST only |
33+
| stl.cpp:115:7:115:7 | stl.cpp:111:16:111:21 | AST only |
34+
| stl.cpp:117:9:117:13 | stl.cpp:111:16:111:21 | AST only |
35+
| stl.cpp:167:13:167:17 | stl.cpp:159:10:159:15 | AST only |
36+
| stl.cpp:171:13:171:17 | stl.cpp:159:10:159:15 | AST only |
37+
| stl.cpp:174:13:174:17 | stl.cpp:159:10:159:15 | AST only |
38+
| stl.cpp:184:7:184:8 | stl.cpp:179:19:179:26 | IR only |
39+
| stl.cpp:185:7:185:8 | stl.cpp:179:19:179:24 | AST only |
40+
| stl.cpp:198:7:198:8 | stl.cpp:190:19:190:24 | AST only |
41+
| stl.cpp:199:7:199:8 | stl.cpp:190:19:190:24 | AST only |
42+
| stl.cpp:221:8:221:9 | stl.cpp:216:18:216:23 | AST only |
43+
| stl.cpp:222:8:222:9 | stl.cpp:217:20:217:25 | AST only |
44+
| stl.cpp:223:8:223:9 | stl.cpp:219:8:219:13 | AST only |
45+
| stl.cpp:242:8:242:9 | stl.cpp:238:32:238:37 | AST only |
46+
| stl.cpp:243:8:243:9 | stl.cpp:240:20:240:25 | AST only |
47+
| stl.cpp:252:8:252:8 | stl.cpp:250:16:250:21 | AST only |
48+
| stl.cpp:260:8:260:8 | stl.cpp:250:16:250:21 | AST only |
49+
| stl.cpp:265:8:265:8 | stl.cpp:263:28:263:33 | AST only |
50+
| stl.cpp:307:8:307:8 | stl.cpp:300:43:300:49 | AST only |
51+
| stl.cpp:315:8:315:8 | stl.cpp:300:43:300:49 | AST only |
52+
| stl.cpp:320:8:320:8 | stl.cpp:300:43:300:49 | AST only |
53+
| stl.cpp:335:11:335:11 | stl.cpp:332:18:332:23 | AST only |
54+
| stl.cpp:336:11:336:11 | stl.cpp:332:18:332:23 | AST only |
55+
| stl.cpp:337:11:337:11 | stl.cpp:332:18:332:23 | AST only |
56+
| stl.cpp:340:11:340:11 | stl.cpp:340:13:340:18 | AST only |
57+
| stl.cpp:349:8:349:9 | stl.cpp:345:18:345:23 | AST only |
58+
| stl.cpp:353:8:353:9 | stl.cpp:345:18:345:23 | AST only |
59+
| stl.cpp:358:8:358:9 | stl.cpp:356:9:356:14 | AST only |
60+
| stl.cpp:362:8:362:9 | stl.cpp:345:18:345:23 | AST only |
61+
| stl.cpp:367:8:367:9 | stl.cpp:365:13:365:18 | AST only |
62+
| stl.cpp:375:8:375:10 | stl.cpp:372:12:372:26 | AST only |
5363
| structlikeclass.cpp:35:8:35:9 | structlikeclass.cpp:29:22:29:27 | AST only |
5464
| structlikeclass.cpp:36:8:36:9 | structlikeclass.cpp:30:24:30:29 | AST only |
5565
| structlikeclass.cpp:37:8:37:9 | structlikeclass.cpp:29:22:29:27 | AST only |

cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.expected

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
| format.cpp:157:7:157:22 | (int)... | format.cpp:147:12:147:25 | call to source |
22
| format.cpp:157:7:157:22 | access to array | format.cpp:147:12:147:25 | call to source |
33
| format.cpp:158:7:158:27 | ... + ... | format.cpp:148:16:148:30 | call to source |
4-
| stl.cpp:101:7:101:7 | (const char *)... | stl.cpp:97:12:97:17 | call to source |
5-
| stl.cpp:101:7:101:7 | a | stl.cpp:97:12:97:17 | call to source |
6-
| stl.cpp:172:7:172:8 | cs | stl.cpp:167:19:167:24 | call to source |
7-
| stl.cpp:172:7:172:8 | cs | stl.cpp:167:19:167:26 | (const char *)... |
4+
| stl.cpp:113:7:113:7 | (const char *)... | stl.cpp:109:12:109:17 | call to source |
5+
| stl.cpp:113:7:113:7 | a | stl.cpp:109:12:109:17 | call to source |
6+
| stl.cpp:184:7:184:8 | cs | stl.cpp:179:19:179:24 | call to source |
7+
| stl.cpp:184:7:184:8 | cs | stl.cpp:179:19:179:26 | (const char *)... |
88
| structlikeclass.cpp:38:8:38:9 | s4 | structlikeclass.cpp:33:8:33:13 | call to source |
99
| structlikeclass.cpp:61:8:61:9 | s2 | structlikeclass.cpp:58:24:58:29 | call to source |
1010
| structlikeclass.cpp:62:8:62:20 | ... = ... | structlikeclass.cpp:62:13:62:18 | call to source |

0 commit comments

Comments
 (0)