Skip to content

Commit 63ff17b

Browse files
geoffw0geoffw0
authored
Merge pull request github#7737 from geoffw0/clrtxt5
C++: Upgrade cpp/cleartext-storage-file
2 parents cc527bd + e42d3e5 commit 63ff17b

File tree

4 files changed

+78
-17
lines changed

4 files changed

+78
-17
lines changed

‎cpp/ql/src/Security/CWE/CWE-311/CleartextFileWrite.ql

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* @name Cleartext storage of sensitive information in file
33
* @description Storing sensitive information in cleartext can expose it
44
* to an attacker.
5-
* @kind problem
5+
* @kind path-problem
66
* @problem.severity warning
77
* @security-severity 7.5
88
* @precision high
@@ -17,6 +17,19 @@ import semmle.code.cpp.security.SensitiveExprs
1717
import semmle.code.cpp.security.FileWrite
1818
import semmle.code.cpp.dataflow.DataFlow
1919
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
20+
import semmle.code.cpp.dataflow.TaintTracking
21+
import DataFlow::PathGraph
22+
23+
/**
24+
* Taint flow from a sensitive expression to a `FileWrite` sink.
25+
*/
26+
class FromSensitiveConfiguration extends TaintTracking::Configuration {
27+
FromSensitiveConfiguration() { this = "FromSensitiveConfiguration" }
28+
29+
override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof SensitiveExpr }
30+
31+
override predicate isSink(DataFlow::Node sink) { any(FileWrite w).getASource() = sink.asExpr() }
32+
}
2033

2134
/**
2235
* An operation on a filename.
@@ -43,12 +56,17 @@ predicate isFileName(GVN gvn) {
4356
)
4457
}
4558

46-
from FileWrite w, SensitiveExpr source, Expr mid, Expr dest
59+
from
60+
FromSensitiveConfiguration config, SensitiveExpr source, DataFlow::PathNode sourceNode, Expr mid,
61+
DataFlow::PathNode midNode, FileWrite w, Expr dest
4762
where
48-
DataFlow::localFlow(DataFlow::exprNode(source), DataFlow::exprNode(mid)) and
63+
config.hasFlowPath(sourceNode, midNode) and
64+
sourceNode.getNode().asExpr() = source and
65+
midNode.getNode().asExpr() = mid and
4966
mid = w.getASource() and
5067
dest = w.getDest() and
5168
not isFileName(globalValueNumber(source)) and // file names are not passwords
5269
not exists(string convChar | convChar = w.getSourceConvChar(mid) | not convChar = ["s", "S"]) // ignore things written with other conversion characters
53-
select w, "This write into file '" + dest.toString() + "' may contain unencrypted data from $@",
54-
source, "this source."
70+
select w, sourceNode, midNode,
71+
"This write into file '" + dest.toString() + "' may contain unencrypted data from $@", source,
72+
"this source."

‎cpp/ql/src/change-notes/2022-01-24-cleartext-storage-file.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The `cpp/cleartext-storage-file` query has been upgraded with non-local taint flow and has been converted to a `path-problem` query.

‎cpp/ql/test/query-tests/Security/CWE/CWE-311/semmle/tests/CleartextFileWrite.expected

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,42 @@
1-
| test2.cpp:43:2:43:8 | call to fprintf | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:43:36:43:43 | password | this source. |
2-
| test2.cpp:44:2:44:8 | call to fprintf | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:44:37:44:45 | thepasswd | this source. |
3-
| test2.cpp:50:2:50:8 | call to fprintf | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:50:41:50:53 | passwd_config | this source. |
4-
| test2.cpp:54:2:54:8 | call to fprintf | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:54:41:54:52 | widepassword | this source. |
5-
| test2.cpp:55:2:55:8 | call to fprintf | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:55:40:55:51 | widepassword | this source. |
6-
| test2.cpp:57:2:57:8 | call to fprintf | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:57:39:57:49 | call to getPassword | this source. |
7-
| test2.cpp:65:3:65:9 | call to fprintf | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:62:18:62:25 | password | this source. |
8-
| test.cpp:45:3:45:7 | call to fputs | This write into file 'file' may contain unencrypted data from $@ | test.cpp:45:9:45:19 | thePassword | this source. |
9-
| test.cpp:70:35:70:35 | call to operator<< | This write into file 'mystream' may contain unencrypted data from $@ | test.cpp:70:38:70:48 | thePassword | this source. |
10-
| test.cpp:73:37:73:41 | call to write | This write into file 'mystream' may contain unencrypted data from $@ | test.cpp:73:43:73:53 | thePassword | this source. |
1+
edges
2+
| test2.cpp:52:44:52:57 | password_tries | test2.cpp:52:40:52:58 | * ... |
3+
| test2.cpp:62:18:62:25 | password | test2.cpp:65:31:65:34 | cpy1 |
4+
| test2.cpp:72:17:72:24 | password | test2.cpp:73:30:73:32 | buf |
5+
| test2.cpp:72:17:72:24 | password | test2.cpp:76:30:76:32 | buf |
6+
| test2.cpp:98:45:98:52 | password | test2.cpp:99:27:99:32 | buffer |
7+
nodes
8+
| test2.cpp:43:36:43:43 | password | semmle.label | password |
9+
| test2.cpp:44:37:44:45 | thepasswd | semmle.label | thepasswd |
10+
| test2.cpp:50:41:50:53 | passwd_config | semmle.label | passwd_config |
11+
| test2.cpp:52:40:52:58 | * ... | semmle.label | * ... |
12+
| test2.cpp:52:44:52:57 | password_tries | semmle.label | password_tries |
13+
| test2.cpp:54:41:54:52 | widepassword | semmle.label | widepassword |
14+
| test2.cpp:55:40:55:51 | widepassword | semmle.label | widepassword |
15+
| test2.cpp:57:39:57:49 | call to getPassword | semmle.label | call to getPassword |
16+
| test2.cpp:62:18:62:25 | password | semmle.label | password |
17+
| test2.cpp:65:31:65:34 | cpy1 | semmle.label | cpy1 |
18+
| test2.cpp:72:17:72:24 | password | semmle.label | password |
19+
| test2.cpp:73:30:73:32 | buf | semmle.label | buf |
20+
| test2.cpp:76:30:76:32 | buf | semmle.label | buf |
21+
| test2.cpp:86:36:86:43 | password | semmle.label | password |
22+
| test2.cpp:91:50:91:63 | passwd_config2 | semmle.label | passwd_config2 |
23+
| test2.cpp:98:45:98:52 | password | semmle.label | password |
24+
| test2.cpp:99:27:99:32 | buffer | semmle.label | buffer |
25+
| test.cpp:45:9:45:19 | thePassword | semmle.label | thePassword |
26+
| test.cpp:70:38:70:48 | thePassword | semmle.label | thePassword |
27+
| test.cpp:73:43:73:53 | thePassword | semmle.label | thePassword |
28+
subpaths
29+
#select
30+
| test2.cpp:43:2:43:8 | call to fprintf | test2.cpp:43:36:43:43 | password | test2.cpp:43:36:43:43 | password | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:43:36:43:43 | password | this source. |
31+
| test2.cpp:44:2:44:8 | call to fprintf | test2.cpp:44:37:44:45 | thepasswd | test2.cpp:44:37:44:45 | thepasswd | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:44:37:44:45 | thepasswd | this source. |
32+
| test2.cpp:50:2:50:8 | call to fprintf | test2.cpp:50:41:50:53 | passwd_config | test2.cpp:50:41:50:53 | passwd_config | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:50:41:50:53 | passwd_config | this source. |
33+
| test2.cpp:54:2:54:8 | call to fprintf | test2.cpp:54:41:54:52 | widepassword | test2.cpp:54:41:54:52 | widepassword | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:54:41:54:52 | widepassword | this source. |
34+
| test2.cpp:55:2:55:8 | call to fprintf | test2.cpp:55:40:55:51 | widepassword | test2.cpp:55:40:55:51 | widepassword | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:55:40:55:51 | widepassword | this source. |
35+
| test2.cpp:57:2:57:8 | call to fprintf | test2.cpp:57:39:57:49 | call to getPassword | test2.cpp:57:39:57:49 | call to getPassword | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:57:39:57:49 | call to getPassword | this source. |
36+
| test2.cpp:65:3:65:9 | call to fprintf | test2.cpp:62:18:62:25 | password | test2.cpp:65:31:65:34 | cpy1 | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:62:18:62:25 | password | this source. |
37+
| test2.cpp:73:3:73:9 | call to fprintf | test2.cpp:72:17:72:24 | password | test2.cpp:73:30:73:32 | buf | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:72:17:72:24 | password | this source. |
38+
| test2.cpp:76:3:76:9 | call to fprintf | test2.cpp:72:17:72:24 | password | test2.cpp:76:30:76:32 | buf | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:72:17:72:24 | password | this source. |
39+
| test2.cpp:99:3:99:9 | call to fprintf | test2.cpp:98:45:98:52 | password | test2.cpp:99:27:99:32 | buffer | This write into file 'log' may contain unencrypted data from $@ | test2.cpp:98:45:98:52 | password | this source. |
40+
| test.cpp:45:3:45:7 | call to fputs | test.cpp:45:9:45:19 | thePassword | test.cpp:45:9:45:19 | thePassword | This write into file 'file' may contain unencrypted data from $@ | test.cpp:45:9:45:19 | thePassword | this source. |
41+
| test.cpp:70:35:70:35 | call to operator<< | test.cpp:70:38:70:48 | thePassword | test.cpp:70:38:70:48 | thePassword | This write into file 'mystream' may contain unencrypted data from $@ | test.cpp:70:38:70:48 | thePassword | this source. |
42+
| test.cpp:73:37:73:41 | call to write | test.cpp:73:43:73:53 | thePassword | test.cpp:73:43:73:53 | thePassword | This write into file 'mystream' may contain unencrypted data from $@ | test.cpp:73:43:73:53 | thePassword | this source. |

‎cpp/ql/test/query-tests/Security/CWE/CWE-311/semmle/tests/test2.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,15 @@ void tests(FILE *log, myStruct &s)
7070
char buf[1024];
7171

7272
strcpy(buf, s.password);
73-
fprintf(log, "buf = %s\n", buf); // BAD [NOT DETECTED]
73+
fprintf(log, "buf = %s\n", buf); // BAD
7474

75+
strcpy(buf, s.password_hash);
76+
fprintf(log, "buf = %s\n", buf); // GOOD [FALSE POSITIVE]
77+
}
78+
79+
{
80+
char buf[1024];
81+
7582
strcpy(buf, s.password_hash);
7683
fprintf(log, "buf = %s\n", buf); // GOOD
7784
}
@@ -89,6 +96,6 @@ void tests(FILE *log, myStruct &s)
8996
char buffer[1024];
9097

9198
snprintf(buffer, 1024, "password = %s", s.password);
92-
fprintf(log, "log: %s", buffer); // BAD [NOT DETECTED]
99+
fprintf(log, "log: %s", buffer); // BAD
93100
}
94101
}

0 commit comments

Comments
 (0)