Skip to content

Commit 91627cb

Browse files
committed
C++: Add models for BSD-style send and recv functions.
1 parent 18225fa commit 91627cb

File tree

9 files changed

+220
-1
lines changed

9 files changed

+220
-1
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,5 @@ private import implementations.Swap
2828
private import implementations.GetDelim
2929
private import implementations.SmartPointer
3030
private import implementations.Sscanf
31+
private import implementations.Send
32+
private import implementations.Recv
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/**
2+
* Provides implementation classes modeling `recv` and various similar
3+
* functions. See `semmle.code.cpp.models.Models` for usage information.
4+
*/
5+
6+
import semmle.code.cpp.models.interfaces.Taint
7+
import semmle.code.cpp.models.interfaces.ArrayFunction
8+
import semmle.code.cpp.models.interfaces.Alias
9+
import semmle.code.cpp.models.interfaces.FlowSource
10+
import semmle.code.cpp.models.interfaces.SideEffect
11+
12+
/** The function `recv` and its assorted variants */
13+
private class Recv extends AliasFunction, ArrayFunction, SideEffectFunction, RemoteFlowFunction {
14+
Recv() {
15+
this.hasGlobalName([
16+
"recv", // recv(socket, dest, len, flags)
17+
"recvfrom", // recvfrom(socket, dest, len, flags, from, fromlen)
18+
"read", // read(socket, dest, len)
19+
"pread" // pread(socket, dest, len, offset)
20+
])
21+
}
22+
23+
override predicate parameterNeverEscapes(int index) {
24+
this.getParameter(index).getUnspecifiedType() instanceof PointerType
25+
}
26+
27+
override predicate parameterEscapesOnlyViaReturn(int index) { none() }
28+
29+
override predicate parameterIsAlwaysReturned(int index) { none() }
30+
31+
override predicate hasArrayWithVariableSize(int bufParam, int countParam) {
32+
bufParam = 1 and countParam = 2
33+
}
34+
35+
override predicate hasArrayInput(int bufParam) { this.hasGlobalName("recvfrom") and bufParam = 4 }
36+
37+
override predicate hasArrayOutput(int bufParam) {
38+
bufParam = 1
39+
or
40+
this.hasGlobalName("recvfrom") and bufParam = 4
41+
}
42+
43+
override predicate hasSpecificReadSideEffect(ParameterIndex i, boolean buffer) {
44+
this.hasGlobalName("recvfrom") and
45+
(
46+
i = 4 and buffer = true
47+
or
48+
i = 5 and buffer = false
49+
)
50+
}
51+
52+
override ParameterIndex getParameterSizeIndex(ParameterIndex i) { i = 1 and result = 2 }
53+
54+
override predicate hasSpecificWriteSideEffect(ParameterIndex i, boolean buffer, boolean mustWrite) {
55+
i = 1 and buffer = true and mustWrite = false
56+
or
57+
this.hasGlobalName("recvfrom") and
58+
(
59+
i = 4 and buffer = true and mustWrite = false
60+
or
61+
i = 5 and buffer = false and mustWrite = false
62+
)
63+
}
64+
65+
override predicate hasOnlySpecificReadSideEffects() { any() }
66+
67+
override predicate hasOnlySpecificWriteSideEffects() { any() }
68+
69+
override predicate hasRemoteFlowSource(FunctionOutput output, string description) {
70+
output.isParameterDeref(1) and
71+
description = "Buffer read by " + this.getName()
72+
}
73+
}
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/**
2+
* Provides implementation classes modeling `send` and various similar
3+
* functions. See `semmle.code.cpp.models.Models` for usage information.
4+
*/
5+
6+
import semmle.code.cpp.models.interfaces.Taint
7+
import semmle.code.cpp.models.interfaces.ArrayFunction
8+
import semmle.code.cpp.models.interfaces.Alias
9+
import semmle.code.cpp.models.interfaces.FlowSource
10+
import semmle.code.cpp.models.interfaces.SideEffect
11+
12+
/** The function `send` and its assorted variants */
13+
private class Send extends AliasFunction, ArrayFunction, SideEffectFunction, RemoteFlowFunctionSink {
14+
Send() {
15+
this.hasGlobalName([
16+
"send", // send(socket, buf, len, flags)
17+
"sendto", // sendto(socket, buf, len, flags, to, tolen)
18+
"write" // write(socket, buf, len);
19+
])
20+
}
21+
22+
override predicate parameterNeverEscapes(int index) {
23+
this.getParameter(index).getUnspecifiedType() instanceof PointerType
24+
}
25+
26+
override predicate parameterEscapesOnlyViaReturn(int index) { none() }
27+
28+
override predicate parameterIsAlwaysReturned(int index) { none() }
29+
30+
override predicate hasArrayWithVariableSize(int bufParam, int countParam) {
31+
bufParam = 1 and countParam = 2
32+
}
33+
34+
override predicate hasArrayInput(int bufParam) { bufParam = 1 }
35+
36+
override predicate hasOnlySpecificReadSideEffects() { any() }
37+
38+
override predicate hasOnlySpecificWriteSideEffects() { any() }
39+
40+
override predicate hasSpecificWriteSideEffect(ParameterIndex i, boolean buffer, boolean mustWrite) {
41+
none()
42+
}
43+
44+
override predicate hasSpecificReadSideEffect(ParameterIndex i, boolean buffer) {
45+
i = 1 and buffer = true
46+
or
47+
exists(this.getParameter(4)) and i = 4 and buffer = false
48+
}
49+
50+
override ParameterIndex getParameterSizeIndex(ParameterIndex i) { i = 1 and result = 2 }
51+
52+
override predicate hasRemoteFlowSink(FunctionInput input, string description) {
53+
input.isParameterDeref(1) and description = "Buffer sent by " + this.getName()
54+
}
55+
}

cpp/ql/src/semmle/code/cpp/models/interfaces/FlowSource.qll

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,12 @@ abstract class LocalFlowFunction extends Function {
2929
*/
3030
abstract predicate hasLocalFlowSource(FunctionOutput output, string description);
3131
}
32+
33+
/** A library function that sends data over a network connection. */
34+
abstract class RemoteFlowFunctionSink extends Function {
35+
/**
36+
* Holds if data described by `description` flows into `input` to a call to this function, and is then
37+
* send over a network connection.
38+
*/
39+
abstract predicate hasRemoteFlowSink(FunctionInput input, string description);
40+
}

cpp/ql/src/semmle/code/cpp/security/FlowSources.qll

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,3 +98,31 @@ private class ArgvSource extends LocalFlowSource {
9898

9999
override string getSourceType() { result = "a command-line argument" }
100100
}
101+
102+
/** A remote data flow sink. */
103+
abstract class RemoteFlowSink extends DataFlow::Node {
104+
/** Gets a string that describes the type of this flow sink. */
105+
abstract string getSinkType();
106+
}
107+
108+
private class RemoteParameterSink extends RemoteFlowSink {
109+
string sourceType;
110+
111+
RemoteParameterSink() {
112+
exists(RemoteFlowFunctionSink func, FunctionInput input, CallInstruction call, int index |
113+
func.hasRemoteFlowSink(input, sourceType) and call.getStaticCallTarget() = func
114+
|
115+
exists(ReadSideEffectInstruction read |
116+
call = read.getPrimaryInstruction() and
117+
read.getIndex() = index and
118+
this.asOperand() = read.getSideEffectOperand() and
119+
input.isParameterDerefOrQualifierObject(index)
120+
)
121+
or
122+
input.isParameterOrQualifierAddress(index) and
123+
this.asOperand() = call.getArgumentOperand(index)
124+
)
125+
}
126+
127+
override string getSinkType() { result = sourceType }
128+
}

cpp/ql/src/semmle/code/cpp/security/Security.qll

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import semmle.code.cpp.exprs.Expr
77
import semmle.code.cpp.commons.Environment
88
import semmle.code.cpp.security.SecurityOptions
9+
import semmle.code.cpp.models.interfaces.FlowSource
910

1011
/**
1112
* Extend this class to customize the security queries for
@@ -60,14 +61,20 @@ class SecurityOptions extends string {
6061
functionCall.getTarget().hasGlobalName(fname) and
6162
exists(functionCall.getArgument(arg)) and
6263
(
63-
fname = ["read", "recv", "recvmsg"] and arg = 1
64+
fname = "recvmsg" and arg = 1
6465
or
6566
fname = "getaddrinfo" and arg = 3
6667
or
6768
fname = "recvfrom" and
6869
(arg = 1 or arg = 4 or arg = 5)
6970
)
7071
)
72+
or
73+
exists(RemoteFlowFunction remote, FunctionOutput output |
74+
functionCall.getTarget() = remote and
75+
output.isParameterDerefOrQualifierObject(arg) and
76+
remote.hasRemoteFlowSource(output, _)
77+
)
7178
}
7279

7380
/**
@@ -81,6 +88,12 @@ class SecurityOptions extends string {
8188
userInputReturn(fname)
8289
)
8390
)
91+
or
92+
exists(RemoteFlowFunction remote, FunctionOutput output |
93+
functionCall.getTarget() = remote and
94+
(output.isReturnValue() or output.isReturnValueDeref()) and
95+
remote.hasRemoteFlowSource(output, _)
96+
)
8497
}
8598

8699
/**

cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/annotate_sinks_only/defaulttainttracking.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,3 +216,22 @@ void test_pointers2()
216216
sink(ptr4); // clean
217217
sink(*ptr4); // $ MISSING: ast,ir
218218
}
219+
220+
// --- recv ---
221+
222+
int recv(int s, char* buf, int len, int flags);
223+
224+
void test_recv() {
225+
char buffer[1024];
226+
recv(0, buffer, sizeof(buffer), 0);
227+
sink(buffer); // $ ast,ir
228+
sink(*buffer); // $ ast,ir
229+
}
230+
231+
// --- send ---
232+
233+
int send(int, const void*, int, int);
234+
235+
void test_send(char* buffer, int length) {
236+
send(0, buffer, length, 0); // $ remote
237+
}

cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/annotate_sinks_only/remote-flow-sink.expected

Whitespace-only changes.
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/** This tests that we are able to detect remote flow sinks. */
2+
3+
import cpp
4+
import TestUtilities.InlineExpectationsTest
5+
import semmle.code.cpp.security.FlowSources
6+
7+
class RemoteFlowSinkTest extends InlineExpectationsTest {
8+
RemoteFlowSinkTest() { this = "RemoteFlowSinkTest" }
9+
10+
override string getARelevantTag() { result = "remote" }
11+
12+
override predicate hasActualResult(Location location, string element, string tag, string value) {
13+
tag = "remote" and
14+
value = "" and
15+
exists(RemoteFlowSink node |
16+
location = node.getLocation() and
17+
element = node.toString()
18+
)
19+
}
20+
}

0 commit comments

Comments
 (0)