Skip to content

Commit 98d73bf

Browse files
authored
Merge pull request github#5072 from MathiasVP/strcrement-model-implementation
C++: Implement model for _strinc and related functions
2 parents aa9ab41 + 07a2075 commit 98d73bf

File tree

4 files changed

+155
-0
lines changed

4 files changed

+155
-0
lines changed

cpp/ql/src/semmle/code/cpp/models/Models.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ private import implementations.Strdup
1717
private import implementations.Strftime
1818
private import implementations.Strtok
1919
private import implementations.Strset
20+
private import implementations.Strcrement
2021
private import implementations.StdContainer
2122
private import implementations.StdPair
2223
private import implementations.StdMap
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/**
2+
* Provides implementation classes modeling `_strinc`, `_strdec` and their variants.
3+
* See `semmle.code.cpp.models.Models` for usage information.
4+
*/
5+
6+
import semmle.code.cpp.models.interfaces.ArrayFunction
7+
import semmle.code.cpp.models.interfaces.Taint
8+
import semmle.code.cpp.models.interfaces.SideEffect
9+
10+
/**
11+
* The function `_strinc`, `_strdec` and their variants.
12+
*/
13+
private class Strcrement extends ArrayFunction, TaintFunction, SideEffectFunction {
14+
Strcrement() {
15+
this.hasGlobalName([
16+
"_strinc", // _strinc(source, locale)
17+
"_wcsinc", // _strinc(source, locale)
18+
"_mbsinc", // _strinc(source)
19+
"_mbsinc_l", // _strinc(source, locale)
20+
"_strdec", // _strdec(start, source)
21+
"_wcsdec", // _wcsdec(start, source)
22+
"_mbsdec", // _mbsdec(start, source)
23+
"_mbsdec_l" // _mbsdec_l(start, source, locale)
24+
])
25+
}
26+
27+
override predicate hasArrayWithNullTerminator(int bufParam) {
28+
// Match all parameters that are not locales.
29+
this.getParameter(bufParam).getUnspecifiedType() instanceof PointerType
30+
}
31+
32+
override predicate hasArrayInput(int bufParam) { hasArrayWithNullTerminator(bufParam) }
33+
34+
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
35+
exists(int index | hasArrayInput(index) |
36+
input.isParameter(index) and output.isReturnValue()
37+
or
38+
input.isParameterDeref(index) and output.isReturnValueDeref()
39+
)
40+
}
41+
42+
override predicate hasOnlySpecificReadSideEffects() { any() }
43+
44+
override predicate hasOnlySpecificWriteSideEffects() { any() }
45+
46+
override predicate hasSpecificReadSideEffect(ParameterIndex i, boolean buffer) {
47+
hasArrayInput(i) and buffer = true
48+
}
49+
}

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5970,6 +5970,68 @@
59705970
| taint.cpp:572:37:572:41 | delim | taint.cpp:572:21:572:26 | call to strsep | TAINT |
59715971
| taint.cpp:573:10:573:18 | ref arg tokenized | taint.cpp:574:11:574:19 | tokenized | |
59725972
| taint.cpp:574:11:574:19 | tokenized | taint.cpp:574:10:574:19 | * ... | TAINT |
5973+
| taint.cpp:584:25:584:30 | source | taint.cpp:585:18:585:23 | source | |
5974+
| taint.cpp:584:39:584:43 | clean | taint.cpp:589:18:589:22 | clean | |
5975+
| taint.cpp:584:82:584:87 | locale | taint.cpp:585:26:585:31 | locale | |
5976+
| taint.cpp:584:82:584:87 | locale | taint.cpp:589:25:589:30 | locale | |
5977+
| taint.cpp:585:10:585:16 | call to _strinc | taint.cpp:585:2:585:32 | ... = ... | |
5978+
| taint.cpp:585:10:585:16 | call to _strinc | taint.cpp:586:7:586:11 | dest1 | |
5979+
| taint.cpp:585:10:585:16 | call to _strinc | taint.cpp:587:8:587:12 | dest1 | |
5980+
| taint.cpp:585:18:585:23 | source | taint.cpp:585:10:585:16 | call to _strinc | TAINT |
5981+
| taint.cpp:585:26:585:31 | locale | taint.cpp:585:10:585:16 | call to _strinc | TAINT |
5982+
| taint.cpp:585:26:585:31 | ref arg locale | taint.cpp:589:25:589:30 | locale | |
5983+
| taint.cpp:586:7:586:11 | ref arg dest1 | taint.cpp:587:8:587:12 | dest1 | |
5984+
| taint.cpp:587:8:587:12 | dest1 | taint.cpp:587:7:587:12 | * ... | TAINT |
5985+
| taint.cpp:589:10:589:16 | call to _strinc | taint.cpp:589:2:589:31 | ... = ... | |
5986+
| taint.cpp:589:10:589:16 | call to _strinc | taint.cpp:590:7:590:11 | dest2 | |
5987+
| taint.cpp:589:10:589:16 | call to _strinc | taint.cpp:591:8:591:12 | dest2 | |
5988+
| taint.cpp:589:18:589:22 | clean | taint.cpp:589:10:589:16 | call to _strinc | TAINT |
5989+
| taint.cpp:589:25:589:30 | locale | taint.cpp:589:10:589:16 | call to _strinc | TAINT |
5990+
| taint.cpp:590:7:590:11 | ref arg dest2 | taint.cpp:591:8:591:12 | dest2 | |
5991+
| taint.cpp:591:8:591:12 | dest2 | taint.cpp:591:7:591:12 | * ... | TAINT |
5992+
| taint.cpp:594:34:594:48 | source_unsigned | taint.cpp:595:26:595:40 | source_unsigned | |
5993+
| taint.cpp:594:57:594:62 | source | taint.cpp:599:40:599:45 | source | |
5994+
| taint.cpp:595:18:595:24 | call to _mbsinc | taint.cpp:595:2:595:41 | ... = ... | |
5995+
| taint.cpp:595:18:595:24 | call to _mbsinc | taint.cpp:596:7:596:19 | dest_unsigned | |
5996+
| taint.cpp:595:18:595:24 | call to _mbsinc | taint.cpp:597:8:597:20 | dest_unsigned | |
5997+
| taint.cpp:595:26:595:40 | source_unsigned | taint.cpp:595:18:595:24 | call to _mbsinc | TAINT |
5998+
| taint.cpp:596:7:596:19 | ref arg dest_unsigned | taint.cpp:597:8:597:20 | dest_unsigned | |
5999+
| taint.cpp:597:8:597:20 | dest_unsigned | taint.cpp:597:7:597:20 | * ... | TAINT |
6000+
| taint.cpp:599:16:599:22 | call to _mbsinc | taint.cpp:599:2:599:46 | ... = ... | |
6001+
| taint.cpp:599:16:599:22 | call to _mbsinc | taint.cpp:600:7:600:10 | dest | |
6002+
| taint.cpp:599:16:599:22 | call to _mbsinc | taint.cpp:601:8:601:11 | dest | |
6003+
| taint.cpp:599:40:599:45 | source | taint.cpp:599:16:599:22 | call to _mbsinc | TAINT |
6004+
| taint.cpp:600:7:600:10 | ref arg dest | taint.cpp:601:8:601:11 | dest | |
6005+
| taint.cpp:601:8:601:11 | dest | taint.cpp:601:7:601:11 | * ... | TAINT |
6006+
| taint.cpp:604:40:604:45 | source | taint.cpp:605:18:605:23 | source | |
6007+
| taint.cpp:604:40:604:45 | source | taint.cpp:605:31:605:36 | source | |
6008+
| taint.cpp:604:40:604:45 | source | taint.cpp:611:25:611:30 | source | |
6009+
| taint.cpp:604:40:604:45 | source | taint.cpp:616:18:616:23 | source | |
6010+
| taint.cpp:604:63:604:67 | clean | taint.cpp:611:18:611:22 | clean | |
6011+
| taint.cpp:604:63:604:67 | clean | taint.cpp:616:26:616:30 | clean | |
6012+
| taint.cpp:605:10:605:16 | call to _strdec | taint.cpp:605:2:605:37 | ... = ... | |
6013+
| taint.cpp:605:10:605:16 | call to _strdec | taint.cpp:606:7:606:11 | dest1 | |
6014+
| taint.cpp:605:10:605:16 | call to _strdec | taint.cpp:607:8:607:12 | dest1 | |
6015+
| taint.cpp:605:18:605:23 | source | taint.cpp:605:18:605:28 | ... + ... | TAINT |
6016+
| taint.cpp:605:18:605:28 | ... + ... | taint.cpp:605:10:605:16 | call to _strdec | TAINT |
6017+
| taint.cpp:605:27:605:28 | 12 | taint.cpp:605:18:605:28 | ... + ... | TAINT |
6018+
| taint.cpp:605:31:605:36 | source | taint.cpp:605:10:605:16 | call to _strdec | TAINT |
6019+
| taint.cpp:606:7:606:11 | ref arg dest1 | taint.cpp:607:8:607:12 | dest1 | |
6020+
| taint.cpp:607:8:607:12 | dest1 | taint.cpp:607:7:607:12 | * ... | TAINT |
6021+
| taint.cpp:611:10:611:16 | call to _strdec | taint.cpp:611:2:611:31 | ... = ... | |
6022+
| taint.cpp:611:10:611:16 | call to _strdec | taint.cpp:612:7:612:11 | dest2 | |
6023+
| taint.cpp:611:10:611:16 | call to _strdec | taint.cpp:613:8:613:12 | dest2 | |
6024+
| taint.cpp:611:18:611:22 | clean | taint.cpp:611:10:611:16 | call to _strdec | TAINT |
6025+
| taint.cpp:611:25:611:30 | source | taint.cpp:611:10:611:16 | call to _strdec | TAINT |
6026+
| taint.cpp:612:7:612:11 | ref arg dest2 | taint.cpp:613:8:613:12 | dest2 | |
6027+
| taint.cpp:613:8:613:12 | dest2 | taint.cpp:613:7:613:12 | * ... | TAINT |
6028+
| taint.cpp:616:10:616:16 | call to _strdec | taint.cpp:616:2:616:31 | ... = ... | |
6029+
| taint.cpp:616:10:616:16 | call to _strdec | taint.cpp:617:7:617:11 | dest3 | |
6030+
| taint.cpp:616:10:616:16 | call to _strdec | taint.cpp:618:8:618:12 | dest3 | |
6031+
| taint.cpp:616:18:616:23 | source | taint.cpp:616:10:616:16 | call to _strdec | TAINT |
6032+
| taint.cpp:616:26:616:30 | clean | taint.cpp:616:10:616:16 | call to _strdec | TAINT |
6033+
| taint.cpp:617:7:617:11 | ref arg dest3 | taint.cpp:618:8:618:12 | dest3 | |
6034+
| taint.cpp:618:8:618:12 | dest3 | taint.cpp:618:7:618:12 | * ... | TAINT |
59736035
| vector.cpp:16:43:16:49 | source1 | vector.cpp:17:26:17:32 | source1 | |
59746036
| vector.cpp:16:43:16:49 | source1 | vector.cpp:31:38:31:44 | source1 | |
59756037
| vector.cpp:17:21:17:33 | call to vector | vector.cpp:19:14:19:14 | v | |

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -574,3 +574,46 @@ void test_strsep(char *source) {
574574
sink(*tokenized); // $ ast,ir
575575
}
576576
}
577+
578+
// --- _strinc and related functions ---
579+
580+
char* _strinc(const char*, _locale_t);
581+
unsigned char* _mbsinc(const unsigned char*);
582+
unsigned char *_strdec(const unsigned char*, const unsigned char*);
583+
584+
void test__strinc(char* source, char* clean, char* dest1, char* dest2, _locale_t locale) {
585+
dest1 = _strinc(source, locale);
586+
sink(dest1); // $ ast,ir
587+
sink(*dest1); // $ ast,ir
588+
589+
dest2 = _strinc(clean, locale);
590+
sink(dest2);
591+
sink(*dest2);
592+
}
593+
594+
void test__mbsinc(unsigned char* source_unsigned, char* source, unsigned char* dest_unsigned, char* dest) {
595+
dest_unsigned = _mbsinc(source_unsigned);
596+
sink(dest_unsigned); // $ ast,ir
597+
sink(*dest_unsigned); // $ ast,ir
598+
599+
dest = (char*)_mbsinc((unsigned char*)source);
600+
sink(dest); // $ ast,ir
601+
sink(*dest); // $ ast,ir
602+
}
603+
604+
void test__strdec(const unsigned char* source, unsigned char* clean, unsigned char* dest1, unsigned char* dest2, unsigned char* dest3) {
605+
dest1 = _strdec(source + 12, source);
606+
sink(dest1); // $ ast,ir
607+
sink(*dest1); // $ ast,ir
608+
609+
// If `clean` does not precede `source` this technically breaks the precondition of _strdec.
610+
// We would still like to have taint, though.
611+
dest2 = _strdec(clean, source);
612+
sink(dest2); // $ ast,ir
613+
sink(*dest2); // $ ast,ir
614+
615+
// Also breaks the precondition on _strdec.
616+
dest3 = _strdec(source, clean);
617+
sink(dest3); // $ ast,ir
618+
sink(*dest3); // $ ast,ir
619+
}

0 commit comments

Comments
 (0)