Skip to content

Commit a3cf721

Browse files
MathiasVPMathiasVP
authored
Merge pull request github#6713 from geoffw0/cwe139
C++: New query for 'Cleartext transmission of sensitive information'
2 parents 799e099 + 679b0f9 commit a3cf721

File tree

10 files changed

+353
-1
lines changed

10 files changed

+353
-1
lines changed

‎cpp/change-notes/2021-06-10-cleartext-transmission.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lgtm,codescanning
2+
* A new query (`cpp/cleartext-transmission`) has been added. This is similar to the `cpp/cleartext-storage-file`, `cpp/cleartext-storage-buffer` and `cpp/cleartext-storage-database` queries but looks for cases where sensitive information is most likely transmitted over a network.

‎cpp/ql/lib/semmle/code/cpp/models/implementations/Recv.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,4 +85,6 @@ private class Recv extends AliasFunction, ArrayFunction, SideEffectFunction,
8585
) and
8686
description = "Buffer read by " + this.getName()
8787
}
88+
89+
override predicate hasSocketInput(FunctionInput input) { input.isParameter(0) }
8890
}

‎cpp/ql/lib/semmle/code/cpp/models/implementations/Send.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,4 +60,6 @@ private class Send extends AliasFunction, ArrayFunction, SideEffectFunction, Rem
6060
override predicate hasRemoteFlowSink(FunctionInput input, string description) {
6161
input.isParameterDeref(1) and description = "Buffer sent by " + this.getName()
6262
}
63+
64+
override predicate hasSocketInput(FunctionInput input) { input.isParameter(0) }
6365
}

‎cpp/ql/lib/semmle/code/cpp/models/interfaces/FlowSource.qll

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ abstract class RemoteFlowSourceFunction extends Function {
1818
* Holds if remote data described by `description` flows from `output` of a call to this function.
1919
*/
2020
abstract predicate hasRemoteFlowSource(FunctionOutput output, string description);
21+
22+
/**
23+
* Holds if remote data from this source comes from a socket described by
24+
* `input`. There is no result if a socket is not specified.
25+
*/
26+
predicate hasSocketInput(FunctionInput input) { none() }
2127
}
2228

2329
/**
@@ -51,4 +57,10 @@ abstract class RemoteFlowSinkFunction extends Function {
5157
* send over a network connection.
5258
*/
5359
abstract predicate hasRemoteFlowSink(FunctionInput input, string description);
60+
61+
/**
62+
* Holds if data put into this sink is transmitted through a socket described
63+
* by `input`. There is no result if a socket is not specified.
64+
*/
65+
predicate hasSocketInput(FunctionInput input) { none() }
5466
}

‎cpp/ql/src/Security/CWE/CWE-311/CleartextStorage.inc.qhelp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ storage.</p>
99
</overview>
1010
<recommendation>
1111

12-
<p>Ensure that sensitive information is always encrypted before being stored, especially before writing to a file.
12+
<p>Ensure that sensitive information is always encrypted before being stored to a file or transmitted over the network.
1313
It may be wise to encrypt information before it is put into a buffer that may be readable in memory.</p>
1414

1515
<p>In general, decrypt sensitive information only at the point where it is necessary for it to be used in

‎cpp/ql/src/Security/CWE/CWE-311/CleartextTransmission.qhelp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<include src="CleartextStorage.inc.qhelp" /></qhelp>

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

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
/**
2+
* @name Cleartext transmission of sensitive information
3+
* @description Transmitting sensitive information across a network in
4+
* cleartext can expose it to an attacker.
5+
* @kind path-problem
6+
* @problem.severity warning
7+
* @security-severity 7.5
8+
* @precision medium
9+
* @id cpp/cleartext-transmission
10+
* @tags security
11+
* external/cwe/cwe-319
12+
*/
13+
14+
import cpp
15+
import semmle.code.cpp.security.SensitiveExprs
16+
import semmle.code.cpp.dataflow.TaintTracking
17+
import semmle.code.cpp.models.interfaces.FlowSource
18+
import DataFlow::PathGraph
19+
20+
/**
21+
* A function call that sends or receives data over a network.
22+
*/
23+
abstract class NetworkSendRecv extends FunctionCall {
24+
/**
25+
* Gets the expression for the socket or similar object used for sending or
26+
* receiving data (if any).
27+
*/
28+
abstract Expr getSocketExpr();
29+
30+
/**
31+
* Gets the expression for the buffer to be sent from / received into.
32+
*/
33+
abstract Expr getDataExpr();
34+
}
35+
36+
/**
37+
* A function call that sends data over a network.
38+
*
39+
* note: functions such as `write` may be writing to a network source or a file. We could attempt to determine which, and sort results into `cpp/cleartext-transmission` and perhaps `cpp/cleartext-storage-file`. In practice it usually isn't very important which query reports a result as long as its reported exactly once.
40+
*/
41+
class NetworkSend extends NetworkSendRecv {
42+
RemoteFlowSinkFunction target;
43+
44+
NetworkSend() { target = this.getTarget() }
45+
46+
override Expr getSocketExpr() {
47+
exists(FunctionInput input, int arg |
48+
target.hasSocketInput(input) and
49+
input.isParameter(arg) and
50+
result = this.getArgument(arg)
51+
)
52+
}
53+
54+
override Expr getDataExpr() {
55+
exists(FunctionInput input, int arg |
56+
target.hasRemoteFlowSink(input, _) and
57+
input.isParameterDeref(arg) and
58+
result = this.getArgument(arg)
59+
)
60+
}
61+
}
62+
63+
/**
64+
* A function call that receives data over a network.
65+
*/
66+
class NetworkRecv extends NetworkSendRecv {
67+
RemoteFlowSourceFunction target;
68+
69+
NetworkRecv() { target = this.getTarget() }
70+
71+
override Expr getSocketExpr() {
72+
exists(FunctionInput input, int arg |
73+
target.hasSocketInput(input) and
74+
input.isParameter(arg) and
75+
result = this.getArgument(arg)
76+
)
77+
}
78+
79+
override Expr getDataExpr() {
80+
exists(FunctionOutput output, int arg |
81+
target.hasRemoteFlowSource(output, _) and
82+
output.isParameterDeref(arg) and
83+
result = this.getArgument(arg)
84+
)
85+
}
86+
}
87+
88+
/**
89+
* Taint flow from a sensitive expression to a network operation with data
90+
* tainted by that expression.
91+
*/
92+
class SensitiveSendRecvConfiguration extends TaintTracking::Configuration {
93+
SensitiveSendRecvConfiguration() { this = "SensitiveSendRecvConfiguration" }
94+
95+
override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof SensitiveExpr }
96+
97+
override predicate isSink(DataFlow::Node sink) {
98+
exists(NetworkSendRecv transmission |
99+
sink.asExpr() = transmission.getDataExpr() and
100+
// a zero socket descriptor is standard input, which is not interesting for this query.
101+
not exists(Zero zero |
102+
DataFlow::localFlow(DataFlow::exprNode(zero),
103+
DataFlow::exprNode(transmission.getSocketExpr()))
104+
)
105+
)
106+
}
107+
}
108+
109+
from
110+
SensitiveSendRecvConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink,
111+
NetworkSendRecv transmission, string msg
112+
where
113+
config.hasFlowPath(source, sink) and
114+
sink.getNode().asExpr() = transmission.getDataExpr() and
115+
if transmission instanceof NetworkSend
116+
then
117+
msg =
118+
"This operation transmits '" + sink.toString() +
119+
"', which may contain unencrypted sensitive data from $@"
120+
else
121+
msg =
122+
"This operation receives into '" + sink.toString() +
123+
"', which may put unencrypted sensitive data into $@"
124+
select transmission, source, sink, msg, source, source.getNode().asExpr().toString()

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
edges
2+
| test3.cpp:68:21:68:29 | password1 | test3.cpp:70:15:70:17 | ptr |
3+
| test3.cpp:75:15:75:22 | password | test3.cpp:77:15:77:17 | ptr |
4+
| test3.cpp:106:20:106:25 | buffer | test3.cpp:108:14:108:19 | buffer |
5+
| test3.cpp:111:28:111:33 | buffer | test3.cpp:113:9:113:14 | buffer |
6+
| test3.cpp:120:9:120:23 | global_password | test3.cpp:138:16:138:29 | call to get_global_str |
7+
| test3.cpp:128:11:128:18 | password | test3.cpp:106:20:106:25 | buffer |
8+
| test3.cpp:132:21:132:22 | call to id | test3.cpp:134:15:134:17 | ptr |
9+
| test3.cpp:132:24:132:32 | password1 | test3.cpp:111:28:111:33 | buffer |
10+
| test3.cpp:132:24:132:32 | password1 | test3.cpp:132:21:132:22 | call to id |
11+
| test3.cpp:138:16:138:29 | call to get_global_str | test3.cpp:140:15:140:18 | data |
12+
| test3.cpp:151:19:151:26 | password | test3.cpp:153:15:153:20 | buffer |
13+
nodes
14+
| test3.cpp:20:15:20:23 | password1 | semmle.label | password1 |
15+
| test3.cpp:24:15:24:23 | password2 | semmle.label | password2 |
16+
| test3.cpp:41:15:41:22 | password | semmle.label | password |
17+
| test3.cpp:49:15:49:22 | password | semmle.label | password |
18+
| test3.cpp:68:21:68:29 | password1 | semmle.label | password1 |
19+
| test3.cpp:70:15:70:17 | ptr | semmle.label | ptr |
20+
| test3.cpp:75:15:75:22 | password | semmle.label | password |
21+
| test3.cpp:77:15:77:17 | ptr | semmle.label | ptr |
22+
| test3.cpp:95:12:95:19 | password | semmle.label | password |
23+
| test3.cpp:106:20:106:25 | buffer | semmle.label | buffer |
24+
| test3.cpp:108:14:108:19 | buffer | semmle.label | buffer |
25+
| test3.cpp:111:28:111:33 | buffer | semmle.label | buffer |
26+
| test3.cpp:113:9:113:14 | buffer | semmle.label | buffer |
27+
| test3.cpp:120:9:120:23 | global_password | semmle.label | global_password |
28+
| test3.cpp:128:11:128:18 | password | semmle.label | password |
29+
| test3.cpp:132:21:132:22 | call to id | semmle.label | call to id |
30+
| test3.cpp:132:24:132:32 | password1 | semmle.label | password1 |
31+
| test3.cpp:134:15:134:17 | ptr | semmle.label | ptr |
32+
| test3.cpp:138:16:138:29 | call to get_global_str | semmle.label | call to get_global_str |
33+
| test3.cpp:140:15:140:18 | data | semmle.label | data |
34+
| test3.cpp:151:19:151:26 | password | semmle.label | password |
35+
| test3.cpp:153:15:153:20 | buffer | semmle.label | buffer |
36+
subpaths
37+
| test3.cpp:132:24:132:32 | password1 | test3.cpp:111:28:111:33 | buffer | test3.cpp:113:9:113:14 | buffer | test3.cpp:132:21:132:22 | call to id |
38+
#select
39+
| test3.cpp:20:3:20:6 | call to send | test3.cpp:20:15:20:23 | password1 | test3.cpp:20:15:20:23 | password1 | This operation transmits 'password1', which may contain unencrypted sensitive data from $@ | test3.cpp:20:15:20:23 | password1 | password1 |
40+
| test3.cpp:24:3:24:6 | call to send | test3.cpp:24:15:24:23 | password2 | test3.cpp:24:15:24:23 | password2 | This operation transmits 'password2', which may contain unencrypted sensitive data from $@ | test3.cpp:24:15:24:23 | password2 | password2 |
41+
| test3.cpp:41:3:41:6 | call to recv | test3.cpp:41:15:41:22 | password | test3.cpp:41:15:41:22 | password | This operation receives into 'password', which may put unencrypted sensitive data into $@ | test3.cpp:41:15:41:22 | password | password |
42+
| test3.cpp:49:3:49:6 | call to recv | test3.cpp:49:15:49:22 | password | test3.cpp:49:15:49:22 | password | This operation receives into 'password', which may put unencrypted sensitive data into $@ | test3.cpp:49:15:49:22 | password | password |
43+
| test3.cpp:70:3:70:6 | call to send | test3.cpp:68:21:68:29 | password1 | test3.cpp:70:15:70:17 | ptr | This operation transmits 'ptr', which may contain unencrypted sensitive data from $@ | test3.cpp:68:21:68:29 | password1 | password1 |
44+
| test3.cpp:77:3:77:6 | call to recv | test3.cpp:75:15:75:22 | password | test3.cpp:77:15:77:17 | ptr | This operation receives into 'ptr', which may put unencrypted sensitive data into $@ | test3.cpp:75:15:75:22 | password | password |
45+
| test3.cpp:95:3:95:6 | call to read | test3.cpp:95:12:95:19 | password | test3.cpp:95:12:95:19 | password | This operation receives into 'password', which may put unencrypted sensitive data into $@ | test3.cpp:95:12:95:19 | password | password |
46+
| test3.cpp:108:2:108:5 | call to recv | test3.cpp:128:11:128:18 | password | test3.cpp:108:14:108:19 | buffer | This operation receives into 'buffer', which may put unencrypted sensitive data into $@ | test3.cpp:128:11:128:18 | password | password |
47+
| test3.cpp:134:3:134:6 | call to send | test3.cpp:132:24:132:32 | password1 | test3.cpp:134:15:134:17 | ptr | This operation transmits 'ptr', which may contain unencrypted sensitive data from $@ | test3.cpp:132:24:132:32 | password1 | password1 |
48+
| test3.cpp:140:3:140:6 | call to send | test3.cpp:120:9:120:23 | global_password | test3.cpp:140:15:140:18 | data | This operation transmits 'data', which may contain unencrypted sensitive data from $@ | test3.cpp:120:9:120:23 | global_password | global_password |
49+
| test3.cpp:153:3:153:6 | call to send | test3.cpp:151:19:151:26 | password | test3.cpp:153:15:153:20 | buffer | This operation transmits 'buffer', which may contain unencrypted sensitive data from $@ | test3.cpp:151:19:151:26 | password | password |

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE/CWE-311/CleartextTransmission.ql

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

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
2+
typedef unsigned long size_t;
3+
#define STDIN_FILENO (0)
4+
5+
size_t strlen(const char *s);
6+
7+
void send(int fd, const void *buf, size_t bufLen, int d);
8+
void recv(int fd, void *buf, size_t bufLen, int d);
9+
void read(int fd, void *buf, size_t bufLen);
10+
11+
void LogonUserA(int a, int b, const char *password, int d, int e, int f);
12+
13+
int val();
14+
15+
void test_send(const char *password1, const char *password2, const char *password_hash, const char *message)
16+
{
17+
{
18+
LogonUserA(val(), val(), password1, val(), val(), val()); // proof `password1` is plaintext
19+
20+
send(val(), password1, strlen(password1), val()); // BAD: `password1` is sent plaintext (certainly)
21+
}
22+
23+
{
24+
send(val(), password2, strlen(password2), val()); // BAD: `password2` is sent plaintext (probably)
25+
}
26+
27+
{
28+
send(val(), password_hash, strlen(password_hash), val()); // GOOD: `password_hash` is sent encrypted
29+
}
30+
31+
{
32+
send(val(), message, strlen(message), val()); // GOOD: `message` is not a password
33+
}
34+
}
35+
36+
void test_receive()
37+
{
38+
{
39+
char password[256];
40+
41+
recv(val(), password, 256, val()); // BAD: `password` is received plaintext (certainly)
42+
43+
LogonUserA(val(), val(), password, val(), val(), val()); // (proof `password` is plaintext)
44+
}
45+
46+
{
47+
char password[256];
48+
49+
recv(val(), password, 256, val()); // BAD: `password` is received plaintext (probably)
50+
}
51+
52+
{
53+
char password_hash[256];
54+
55+
recv(val(), password_hash, 256, val()); // GOOD: `password` is received encrypted
56+
}
57+
58+
{
59+
char message[256];
60+
61+
recv(val(), message, 256, val()); // GOOD: `message` is not a password
62+
}
63+
}
64+
65+
void test_dataflow(const char *password1)
66+
{
67+
{
68+
const char *ptr = password1;
69+
70+
send(val(), ptr, strlen(ptr), val()); // BAD: `password` is sent plaintext
71+
}
72+
73+
{
74+
char password[256];
75+
char *ptr = password;
76+
77+
recv(val(), ptr, 256, val()); // BAD: `password` is received plaintext
78+
}
79+
80+
{
81+
char buffer[256];
82+
83+
recv(val(), buffer, 256, val()); // BAD: `password` is received plaintext [NOT DETECTED]
84+
85+
char *password = buffer;
86+
}
87+
}
88+
89+
void test_read()
90+
{
91+
{
92+
char password[256];
93+
int fd = val();
94+
95+
read(fd, password, 256); // BAD: `password` is received plaintext
96+
}
97+
98+
{
99+
char password[256];
100+
int fd = STDIN_FILENO;
101+
102+
read(fd, password, 256); // GOOD: `password` is received from stdin, not a network socket
103+
}
104+
}
105+
106+
void my_recv(char *buffer, size_t bufferSize)
107+
{
108+
recv(val(), buffer, bufferSize, val());
109+
}
110+
111+
const char *id(const char *buffer)
112+
{
113+
return buffer;
114+
}
115+
116+
char *global_password;
117+
118+
char *get_global_str()
119+
{
120+
return global_password;
121+
}
122+
123+
void test_interprocedural(const char *password1)
124+
{
125+
{
126+
char password[256];
127+
128+
my_recv(password, 256); // BAD: `password` is received plaintext [detected on line 108]
129+
}
130+
131+
{
132+
const char *ptr = id(password1);
133+
134+
send(val(), ptr, strlen(ptr), val()); // BAD: `password1` is sent plaintext
135+
}
136+
137+
{
138+
char *data = get_global_str();
139+
140+
send(val(), data, strlen(data), val()); // BAD: `global_password` is sent plaintext
141+
}
142+
}
143+
144+
char *strncpy(char *s1, const char *s2, size_t n);
145+
146+
void test_taint(const char *password)
147+
{
148+
{
149+
char buffer[16];
150+
151+
strncpy(buffer, password, 16);
152+
buffer[15] = 0;
153+
send(val(), buffer, 16, val()); // BAD: `password` is (partially) sent plaintext
154+
}
155+
}

0 commit comments

Comments
 (0)