Skip to content

Commit fa26b55

Browse files
geoffw0geoffw0
committed
C++: Add models-as-data models for ZMQ networking library + wiring.
1 parent 16f9ad0 commit fa26b55

7 files changed

Lines changed: 69 additions & 10 deletions

File tree

‎cpp/ql/lib/semmle/code/cpp/models/Models.qll‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,3 +41,4 @@ private import implementations.SqLite3
4141
private import implementations.PostgreSql
4242
private import implementations.System
4343
private import implementations.StructuredExceptionHandling
44+
private import implementations.ZMQ

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/**
2+
* Provides implementation classes modeling the ZeroMQ networking library.
3+
*/
4+
5+
import semmle.code.cpp.models.interfaces.FlowSource
6+
7+
/**
8+
* Remote flow sources.
9+
*/
10+
private class ZMQSource extends SourceModelCsv {
11+
override predicate row(string row) {
12+
row =
13+
[
14+
";;false;zmq_recv;;;Argument[*1];remote",
15+
";;false;zmq_recvmsg;;;Argument[*1];remote",
16+
";;false;zmq_msg_recv;;;Argument[*0];remote",
17+
]
18+
}
19+
}
20+
21+
/**
22+
* Remote flow sinks.
23+
*/
24+
private class ZMQSinks extends SinkModelCsv {
25+
override predicate row(string row) {
26+
row =
27+
[
28+
";;false;zmq_send;;;Argument[*1];remote-sink",
29+
";;false;zmq_msg_init_data;;;Argument[*1];remote-sink",
30+
";;false;zmq_sendmsg;;;Argument[*1];remote-sink",
31+
";;false;zmq_msg_send;;;Argument[*0];remote-sink",
32+
]
33+
}
34+
}
35+
36+
// TODO: flow into / through zmq_msg_data ?

‎cpp/ql/lib/semmle/code/cpp/security/FlowSources.qll‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,9 @@ abstract class RemoteFlowSink extends DataFlow::Node {
9393
abstract string getSinkType();
9494
}
9595

96+
/**
97+
* A remote flow sink derived from the `RemoteFlowSinkFunction` model.
98+
*/
9699
private class RemoteParameterSink extends RemoteFlowSink {
97100
string sourceType;
98101

@@ -106,3 +109,12 @@ private class RemoteParameterSink extends RemoteFlowSink {
106109

107110
override string getSinkType() { result = sourceType }
108111
}
112+
113+
/**
114+
* A remote flow sink defined in a CSV model.
115+
*/
116+
private class RemoteFlowFromCSVSink extends RemoteFlowSink {
117+
RemoteFlowFromCSVSink() { sinkNode(this, "remote-sink") }
118+
119+
override string getSinkType() { result = "remote flow sink" }
120+
}

‎cpp/ql/src/Security/CWE/CWE-497/ExposedSystemData.ql‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,20 @@ import cpp
1616
import semmle.code.cpp.ir.dataflow.TaintTracking
1717
import semmle.code.cpp.models.interfaces.FlowSource
1818
import semmle.code.cpp.models.implementations.Memset
19+
import semmle.code.cpp.security.FlowSources
1920
import ExposedSystemData::PathGraph
2021
import SystemData
2122

2223
module ExposedSystemDataConfig implements DataFlow::ConfigSig {
2324
predicate isSource(DataFlow::Node source) { source = any(SystemData sd).getAnExpr() }
2425

2526
predicate isSink(DataFlow::Node sink) {
26-
exists(FunctionCall fc, FunctionInput input, int arg |
27-
fc.getTarget().(RemoteFlowSinkFunction).hasRemoteFlowSink(input, _) and
28-
input.isParameterDeref(arg) and
29-
fc.getArgument(arg).getAChild*() = sink.asIndirectExpr()
27+
sink instanceof RemoteFlowSink
28+
or
29+
// workaround for cases where the sink contains the tainted thing as a child; this could
30+
// probably be handled better with taint inheriting content or similar modelling.
31+
exists(RemoteFlowSink sinkNode |
32+
sinkNode.asIndirectExpr().getAChild*() = sink.asIndirectExpr()
3033
)
3134
}
3235

‎cpp/ql/test/library-tests/dataflow/source-sink-tests/sources-and-sinks.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,17 +101,17 @@ void test_zmc(void *socket) {
101101
zmq_msg_t msg1, msg2;
102102
char buffer[1024];
103103

104-
if (zmq_recv(socket, buffer, sizeof(buffer), 0) >= 0) { // $ MISSING: remote_source
104+
if (zmq_recv(socket, buffer, sizeof(buffer), 0) >= 0) { // $ remote_source
105105
// ...
106106
}
107107

108108
zmq_msg_init(&msg1);
109-
if (zmq_msg_recv(&msg1, socket, 0) >= 0) { // $ MISSING: remote_source
109+
if (zmq_msg_recv(&msg1, socket, 0) >= 0) { // $ remote_source
110110
// ...
111111
}
112112

113113
zmq_msg_init(&msg2);
114-
if (zmq_recvmsg(socket, &msg2, 0) >= 0) { // $ MISSING: remote_source
114+
if (zmq_recvmsg(socket, &msg2, 0) >= 0) { // $ remote_source
115115
// ...
116116
}
117117
}

‎cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/ExposedSystemData.expected‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ edges
1010
| tests2.cpp:111:14:111:15 | *c1 [*ptr] | tests2.cpp:111:14:111:19 | *ptr | provenance | |
1111
| tests2.cpp:111:14:111:15 | *c1 [*ptr] | tests2.cpp:111:17:111:19 | *ptr | provenance | |
1212
| tests2.cpp:111:17:111:19 | *ptr | tests2.cpp:111:14:111:19 | *ptr | provenance | |
13+
| tests2.cpp:134:17:134:22 | *call to getenv | tests2.cpp:138:23:138:34 | *message_data | provenance | |
14+
| tests2.cpp:134:17:134:22 | *call to getenv | tests2.cpp:143:34:143:45 | *message_data | provenance | |
1315
| tests_sockets.cpp:26:15:26:20 | *call to getenv | tests_sockets.cpp:39:19:39:22 | *path | provenance | |
1416
| tests_sockets.cpp:26:15:26:20 | *call to getenv | tests_sockets.cpp:43:20:43:23 | *path | provenance | |
1517
| tests_sockets.cpp:63:15:63:20 | *call to getenv | tests_sockets.cpp:76:19:76:22 | *path | provenance | |
@@ -36,6 +38,9 @@ nodes
3638
| tests2.cpp:111:14:111:15 | *c1 [*ptr] | semmle.label | *c1 [*ptr] |
3739
| tests2.cpp:111:14:111:19 | *ptr | semmle.label | *ptr |
3840
| tests2.cpp:111:17:111:19 | *ptr | semmle.label | *ptr |
41+
| tests2.cpp:134:17:134:22 | *call to getenv | semmle.label | *call to getenv |
42+
| tests2.cpp:138:23:138:34 | *message_data | semmle.label | *message_data |
43+
| tests2.cpp:143:34:143:45 | *message_data | semmle.label | *message_data |
3944
| tests_sockets.cpp:26:15:26:20 | *call to getenv | semmle.label | *call to getenv |
4045
| tests_sockets.cpp:39:19:39:22 | *path | semmle.label | *path |
4146
| tests_sockets.cpp:43:20:43:23 | *path | semmle.label | *path |
@@ -56,6 +61,8 @@ subpaths
5661
| tests2.cpp:93:14:93:17 | *str1 | tests2.cpp:91:42:91:45 | *str1 | tests2.cpp:93:14:93:17 | *str1 | This operation exposes system data from $@. | tests2.cpp:91:42:91:45 | *str1 | *str1 |
5762
| tests2.cpp:102:14:102:15 | *pw | tests2.cpp:101:8:101:15 | *call to getpwuid | tests2.cpp:102:14:102:15 | *pw | This operation exposes system data from $@. | tests2.cpp:101:8:101:15 | *call to getpwuid | *call to getpwuid |
5863
| tests2.cpp:111:14:111:19 | *ptr | tests2.cpp:109:12:109:17 | *call to getenv | tests2.cpp:111:14:111:19 | *ptr | This operation exposes system data from $@. | tests2.cpp:109:12:109:17 | *call to getenv | *call to getenv |
64+
| tests2.cpp:138:23:138:34 | *message_data | tests2.cpp:134:17:134:22 | *call to getenv | tests2.cpp:138:23:138:34 | *message_data | This operation exposes system data from $@. | tests2.cpp:134:17:134:22 | *call to getenv | *call to getenv |
65+
| tests2.cpp:143:34:143:45 | *message_data | tests2.cpp:134:17:134:22 | *call to getenv | tests2.cpp:143:34:143:45 | *message_data | This operation exposes system data from $@. | tests2.cpp:134:17:134:22 | *call to getenv | *call to getenv |
5966
| tests_sockets.cpp:39:19:39:22 | *path | tests_sockets.cpp:26:15:26:20 | *call to getenv | tests_sockets.cpp:39:19:39:22 | *path | This operation exposes system data from $@. | tests_sockets.cpp:26:15:26:20 | *call to getenv | *call to getenv |
6067
| tests_sockets.cpp:43:20:43:23 | *path | tests_sockets.cpp:26:15:26:20 | *call to getenv | tests_sockets.cpp:43:20:43:23 | *path | This operation exposes system data from $@. | tests_sockets.cpp:26:15:26:20 | *call to getenv | *call to getenv |
6168
| tests_sockets.cpp:76:19:76:22 | *path | tests_sockets.cpp:63:15:63:20 | *call to getenv | tests_sockets.cpp:76:19:76:22 | *path | This operation exposes system data from $@. | tests_sockets.cpp:63:15:63:20 | *call to getenv | *call to getenv |

‎cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/tests2.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,16 +135,16 @@ void test_zmq(void *remoteSocket)
135135
message_len = strlen(message_data) + 1;
136136

137137
// send as data
138-
if (zmq_send(socket, message_data, message_len, 0) >= 0) { // BAD: outputs HOME environment variable [NOT DETECTED]
138+
if (zmq_send(socket, message_data, message_len, 0) >= 0) { // BAD: outputs HOME environment variable
139139
// ...
140140
}
141141

142142
// send as message
143143
if (zmq_msg_init_data(&message, message_data, message_len, 0, 0)) {
144-
if (zmq_sendmsg(remoteSocket, &message, message_len)) { // BAD: outputs HOME environment variable [NOT DETECTED]
144+
if (zmq_sendmsg(remoteSocket, &message, message_len)) { // BAD: outputs HOME environment variable (detected above)
145145
// ...
146146
}
147-
if (zmq_msg_send(&message, remoteSocket, message_len)) { // BAD: outputs HOME environment variable [NOT DETECTED]
147+
if (zmq_msg_send(&message, remoteSocket, message_len)) { // BAD: outputs HOME environment variable (detected above)
148148
// ...
149149
}
150150
}

0 commit comments

Comments
 (0)