Skip to content

Commit f0ce524

Browse files
authored
Merge pull request github#5147 from MathiasVP/model-bsd-sockets-part-1
C++: Add models for BSD-style send and recv functions
2 parents 7e83a60 + 729c7f2 commit f0ce524

File tree

14 files changed

+266
-22
lines changed

14 files changed

+266
-22
lines changed

cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsSpecific.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ class UntrustedDataToExternalAPIConfig extends TaintTracking::Configuration {
4646
UntrustedDataToExternalAPIConfig() { this = "UntrustedDataToExternalAPIConfig" }
4747

4848
override predicate isSource(DataFlow::Node source) {
49-
exists(RemoteFlowFunction remoteFlow |
49+
exists(RemoteFlowSourceFunction remoteFlow |
5050
remoteFlow = source.asExpr().(Call).getTarget() and
5151
remoteFlow.hasRemoteFlowSource(_, _)
5252
)

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

cpp/ql/src/semmle/code/cpp/models/implementations/Fread.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import semmle.code.cpp.models.interfaces.Alias
22
import semmle.code.cpp.models.interfaces.FlowSource
33

4-
private class Fread extends AliasFunction, RemoteFlowFunction {
4+
private class Fread extends AliasFunction, RemoteFlowSourceFunction {
55
Fread() { this.hasGlobalName("fread") }
66

77
override predicate parameterNeverEscapes(int n) {

cpp/ql/src/semmle/code/cpp/models/implementations/GetDelim.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import semmle.code.cpp.models.interfaces.FlowSource
77
* The standard functions `getdelim`, `getwdelim` and the glibc variant `__getdelim`.
88
*/
99
private class GetDelimFunction extends TaintFunction, AliasFunction, SideEffectFunction,
10-
RemoteFlowFunction {
10+
RemoteFlowSourceFunction {
1111
GetDelimFunction() { hasGlobalName(["getdelim", "getwdelim", "__getdelim"]) }
1212

1313
override predicate hasTaintFlow(FunctionInput i, FunctionOutput o) {

cpp/ql/src/semmle/code/cpp/models/implementations/Getenv.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import semmle.code.cpp.models.interfaces.FlowSource
88
/**
99
* The POSIX function `getenv`.
1010
*/
11-
class Getenv extends LocalFlowFunction {
11+
class Getenv extends LocalFlowSourceFunction {
1212
Getenv() { this.hasGlobalName("getenv") }
1313

1414
override predicate hasLocalFlowSource(FunctionOutput output, string description) {

cpp/ql/src/semmle/code/cpp/models/implementations/Gets.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import semmle.code.cpp.models.interfaces.FlowSource
1414
* The standard functions `gets` and `fgets`.
1515
*/
1616
private class GetsFunction extends DataFlowFunction, TaintFunction, ArrayFunction, AliasFunction,
17-
SideEffectFunction, RemoteFlowFunction {
17+
SideEffectFunction, RemoteFlowSourceFunction {
1818
GetsFunction() {
1919
// gets(str)
2020
// fgets(str, num, stream)
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
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,
14+
RemoteFlowSourceFunction {
15+
Recv() {
16+
this.hasGlobalName([
17+
"recv", // recv(socket, dest, len, flags)
18+
"recvfrom", // recvfrom(socket, dest, len, flags, from, fromlen)
19+
"recvmsg", // recvmsg(socket, msg, flags)
20+
"read", // read(socket, dest, len)
21+
"pread" // pread(socket, dest, len, offset)
22+
])
23+
}
24+
25+
override predicate parameterNeverEscapes(int index) {
26+
this.getParameter(index).getUnspecifiedType() instanceof PointerType
27+
}
28+
29+
override predicate parameterEscapesOnlyViaReturn(int index) { none() }
30+
31+
override predicate parameterIsAlwaysReturned(int index) { none() }
32+
33+
override predicate hasArrayWithVariableSize(int bufParam, int countParam) {
34+
not this.hasGlobalName("recvmsg") and
35+
bufParam = 1 and
36+
countParam = 2
37+
}
38+
39+
override predicate hasArrayInput(int bufParam) { this.hasGlobalName("recvfrom") and bufParam = 4 }
40+
41+
override predicate hasArrayOutput(int bufParam) {
42+
bufParam = 1
43+
or
44+
this.hasGlobalName("recvfrom") and bufParam = 4
45+
}
46+
47+
override predicate hasSpecificReadSideEffect(ParameterIndex i, boolean buffer) {
48+
this.hasGlobalName("recvfrom") and
49+
(
50+
i = 4 and buffer = true
51+
or
52+
i = 5 and buffer = false
53+
)
54+
or
55+
this.hasGlobalName("recvmsg") and
56+
i = 1 and
57+
buffer = true
58+
}
59+
60+
override ParameterIndex getParameterSizeIndex(ParameterIndex i) { i = 1 and result = 2 }
61+
62+
override predicate hasSpecificWriteSideEffect(ParameterIndex i, boolean buffer, boolean mustWrite) {
63+
i = 1 and buffer = true and mustWrite = false
64+
or
65+
this.hasGlobalName("recvfrom") and
66+
(
67+
i = 4 and buffer = true and mustWrite = false
68+
or
69+
i = 5 and buffer = false and mustWrite = false
70+
)
71+
}
72+
73+
override predicate hasOnlySpecificReadSideEffects() { any() }
74+
75+
override predicate hasOnlySpecificWriteSideEffects() { any() }
76+
77+
override predicate hasRemoteFlowSource(FunctionOutput output, string description) {
78+
(
79+
output.isParameterDeref(1)
80+
or
81+
this.hasGlobalName("recvfrom") and output.isParameterDeref([4, 5])
82+
) and
83+
description = "Buffer read by " + this.getName()
84+
}
85+
}
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
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, RemoteFlowSinkFunction {
14+
Send() {
15+
this.hasGlobalName([
16+
"send", // send(socket, buf, len, flags)
17+
"sendto", // sendto(socket, buf, len, flags, to, tolen)
18+
"sendmsg", // sendmsg(socket, msg, flags)
19+
"write" // write(socket, buf, len);
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+
not this.hasGlobalName("sendmsg") and
33+
bufParam = 1 and
34+
countParam = 2
35+
}
36+
37+
override predicate hasArrayInput(int bufParam) { bufParam = 1 }
38+
39+
override predicate hasOnlySpecificReadSideEffects() { any() }
40+
41+
override predicate hasOnlySpecificWriteSideEffects() { any() }
42+
43+
override predicate hasSpecificWriteSideEffect(ParameterIndex i, boolean buffer, boolean mustWrite) {
44+
none()
45+
}
46+
47+
override predicate hasSpecificReadSideEffect(ParameterIndex i, boolean buffer) {
48+
i = 1 and buffer = true
49+
or
50+
this.hasGlobalName("sendto") and i = 4 and buffer = false
51+
or
52+
this.hasGlobalName("sendmsg") and i = 1 and buffer = true
53+
}
54+
55+
override ParameterIndex getParameterSizeIndex(ParameterIndex i) { i = 1 and result = 2 }
56+
57+
override predicate hasRemoteFlowSink(FunctionInput input, string description) {
58+
input.isParameterDeref(1) and description = "Buffer sent by " + this.getName()
59+
}
60+
}
Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
/**
2-
* Provides a class for modeling functions that return data from potentially untrusted sources. To use
3-
* this QL library, create a QL class extending `DataFlowFunction` with a
2+
* Provides classes for modeling functions that return data from (or send data to) potentially untrusted
3+
* sources. To use this QL library, create a QL class extending `DataFlowFunction` with a
44
* characteristic predicate that selects the function or set of functions you
55
* are modeling. Within that class, override the predicates provided by
6-
* `RemoteFlowFunction` to match the flow within that function.
6+
* `RemoteFlowSourceFunction` or `RemoteFlowSinkFunction` to match the flow within that function.
77
*/
88

99
import cpp
@@ -13,19 +13,42 @@ import semmle.code.cpp.models.Models
1313
/**
1414
* A library function that returns data that may be read from a network connection.
1515
*/
16-
abstract class RemoteFlowFunction extends Function {
16+
abstract class RemoteFlowSourceFunction extends Function {
1717
/**
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);
2121
}
2222

23+
/**
24+
* DEPRECATED: Use `RemoteFlowSourceFunction` instead.
25+
*
26+
* A library function that returns data that may be read from a network connection.
27+
*/
28+
deprecated class RemoteFlowFunction = RemoteFlowSourceFunction;
29+
2330
/**
2431
* A library function that returns data that is directly controlled by a user.
2532
*/
26-
abstract class LocalFlowFunction extends Function {
33+
abstract class LocalFlowSourceFunction extends Function {
2734
/**
2835
* Holds if data described by `description` flows from `output` of a call to this function.
2936
*/
3037
abstract predicate hasLocalFlowSource(FunctionOutput output, string description);
3138
}
39+
40+
/**
41+
* DEPRECATED: Use `LocalFlowSourceFunction` instead.
42+
*
43+
* A library function that returns data that is directly controlled by a user.
44+
*/
45+
deprecated class LocalFlowFunction = LocalFlowSourceFunction;
46+
47+
/** A library function that sends data over a network connection. */
48+
abstract class RemoteFlowSinkFunction extends Function {
49+
/**
50+
* Holds if data described by `description` flows into `input` to a call to this function, and is then
51+
* send over a network connection.
52+
*/
53+
abstract predicate hasRemoteFlowSink(FunctionInput input, string description);
54+
}

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

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ private class RemoteReturnSource extends RemoteFlowSource {
2323
string sourceType;
2424

2525
RemoteReturnSource() {
26-
exists(RemoteFlowFunction func, CallInstruction instr, FunctionOutput output |
26+
exists(RemoteFlowSourceFunction func, CallInstruction instr, FunctionOutput output |
2727
asInstruction() = instr and
2828
instr.getStaticCallTarget() = func and
2929
func.hasRemoteFlowSource(output, sourceType) and
@@ -42,7 +42,7 @@ private class RemoteParameterSource extends RemoteFlowSource {
4242
string sourceType;
4343

4444
RemoteParameterSource() {
45-
exists(RemoteFlowFunction func, WriteSideEffectInstruction instr, FunctionOutput output |
45+
exists(RemoteFlowSourceFunction func, WriteSideEffectInstruction instr, FunctionOutput output |
4646
asInstruction() = instr and
4747
instr.getPrimaryInstruction().(CallInstruction).getStaticCallTarget() = func and
4848
func.hasRemoteFlowSource(output, sourceType) and
@@ -57,7 +57,7 @@ private class LocalReturnSource extends LocalFlowSource {
5757
string sourceType;
5858

5959
LocalReturnSource() {
60-
exists(LocalFlowFunction func, CallInstruction instr, FunctionOutput output |
60+
exists(LocalFlowSourceFunction func, CallInstruction instr, FunctionOutput output |
6161
asInstruction() = instr and
6262
instr.getStaticCallTarget() = func and
6363
func.hasLocalFlowSource(output, sourceType) and
@@ -76,7 +76,7 @@ private class LocalParameterSource extends LocalFlowSource {
7676
string sourceType;
7777

7878
LocalParameterSource() {
79-
exists(LocalFlowFunction func, WriteSideEffectInstruction instr, FunctionOutput output |
79+
exists(LocalFlowSourceFunction func, WriteSideEffectInstruction instr, FunctionOutput output |
8080
asInstruction() = instr and
8181
instr.getPrimaryInstruction().(CallInstruction).getStaticCallTarget() = func and
8282
func.hasLocalFlowSource(output, sourceType) and
@@ -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(RemoteFlowSinkFunction 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+
}

0 commit comments

Comments
 (0)