C++: Address review comments.

Author: MathiasVP

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

@@ -46,7 +46,7 @@ class UntrustedDataToExternalAPIConfig extends TaintTracking::Configuration {
   UntrustedDataToExternalAPIConfig() { this = "UntrustedDataToExternalAPIConfig" }
 
   override predicate isSource(DataFlow::Node source) {
-    exists(RemoteFlowFunction remoteFlow |
+    exists(RemoteFlowSourceFunction remoteFlow |
       remoteFlow = source.asExpr().(Call).getTarget() and
       remoteFlow.hasRemoteFlowSource(_, _)
     )

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

@@ -1,7 +1,7 @@
 import semmle.code.cpp.models.interfaces.Alias
 import semmle.code.cpp.models.interfaces.FlowSource
 
-private class Fread extends AliasFunction, RemoteFlowFunction {
+private class Fread extends AliasFunction, RemoteFlowSourceFunction {
   Fread() { this.hasGlobalName("fread") }
 
   override predicate parameterNeverEscapes(int n) {

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

@@ -7,7 +7,7 @@ import semmle.code.cpp.models.interfaces.FlowSource
  * The standard functions `getdelim`, `getwdelim` and the glibc variant `__getdelim`.
  */
 private class GetDelimFunction extends TaintFunction, AliasFunction, SideEffectFunction,
-  RemoteFlowFunction {
+  RemoteFlowSourceFunction {
   GetDelimFunction() { hasGlobalName(["getdelim", "getwdelim", "__getdelim"]) }
 
   override predicate hasTaintFlow(FunctionInput i, FunctionOutput o) {

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

@@ -8,7 +8,7 @@ import semmle.code.cpp.models.interfaces.FlowSource
 /**
  * The POSIX function `getenv`.
  */
-class Getenv extends LocalFlowFunction {
+class Getenv extends LocalFlowSourceFunction {
   Getenv() { this.hasGlobalName("getenv") }
 
   override predicate hasLocalFlowSource(FunctionOutput output, string description) {

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

@@ -14,7 +14,7 @@ import semmle.code.cpp.models.interfaces.FlowSource
  * The standard functions `gets` and `fgets`.
  */
 private class GetsFunction extends DataFlowFunction, TaintFunction, ArrayFunction, AliasFunction,
-  SideEffectFunction, RemoteFlowFunction {
+  SideEffectFunction, RemoteFlowSourceFunction {
   GetsFunction() {
     // gets(str)
     // fgets(str, num, stream)

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

@@ -10,11 +10,13 @@ import semmle.code.cpp.models.interfaces.FlowSource
 import semmle.code.cpp.models.interfaces.SideEffect
 
 /** The function `recv` and its assorted variants */
-private class Recv extends AliasFunction, ArrayFunction, SideEffectFunction, RemoteFlowFunction {
+private class Recv extends AliasFunction, ArrayFunction, SideEffectFunction,
+  RemoteFlowSourceFunction {
   Recv() {
     this.hasGlobalName([
         "recv", // recv(socket, dest, len, flags)
         "recvfrom", // recvfrom(socket, dest, len, flags, from, fromlen)
+        "recvmsg", // recvmsg(socket, msg, flags)
         "read", // read(socket, dest, len)
         "pread" // pread(socket, dest, len, offset)
       ])
@@ -29,7 +31,9 @@ private class Recv extends AliasFunction, ArrayFunction, SideEffectFunction, Rem
   override predicate parameterIsAlwaysReturned(int index) { none() }
 
   override predicate hasArrayWithVariableSize(int bufParam, int countParam) {
-    bufParam = 1 and countParam = 2
+    not this.hasGlobalName("recvmsg") and
+    bufParam = 1 and
+    countParam = 2
   }
 
   override predicate hasArrayInput(int bufParam) { this.hasGlobalName("recvfrom") and bufParam = 4 }
@@ -47,6 +51,10 @@ private class Recv extends AliasFunction, ArrayFunction, SideEffectFunction, Rem
       or
       i = 5 and buffer = false
     )
+    or
+    this.hasGlobalName("recvmsg") and
+    i = 1 and
+    buffer = true
   }
 
   override ParameterIndex getParameterSizeIndex(ParameterIndex i) { i = 1 and result = 2 }
@@ -67,7 +75,11 @@ private class Recv extends AliasFunction, ArrayFunction, SideEffectFunction, Rem
   override predicate hasOnlySpecificWriteSideEffects() { any() }
 
   override predicate hasRemoteFlowSource(FunctionOutput output, string description) {
-    output.isParameterDeref(1) and
+    (
+      output.isParameterDeref(1)
+      or
+      this.hasGlobalName("recvfrom") and output.isParameterDeref([4, 5])
+    ) and
     description = "Buffer read by " + this.getName()
   }
 }

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

@@ -10,11 +10,12 @@ import semmle.code.cpp.models.interfaces.FlowSource
 import semmle.code.cpp.models.interfaces.SideEffect
 
 /** The function `send` and its assorted variants */
-private class Send extends AliasFunction, ArrayFunction, SideEffectFunction, RemoteFlowFunctionSink {
+private class Send extends AliasFunction, ArrayFunction, SideEffectFunction, RemoteFlowSinkFunction {
   Send() {
     this.hasGlobalName([
         "send", // send(socket, buf, len, flags)
         "sendto", // sendto(socket, buf, len, flags, to, tolen)
+        "sendmsg", // sendmsg(socket, msg, flags)
         "write" // write(socket, buf, len);
       ])
   }
@@ -28,7 +29,9 @@ private class Send extends AliasFunction, ArrayFunction, SideEffectFunction, Rem
   override predicate parameterIsAlwaysReturned(int index) { none() }
 
   override predicate hasArrayWithVariableSize(int bufParam, int countParam) {
-    bufParam = 1 and countParam = 2
+    not this.hasGlobalName("sendmsg") and
+    bufParam = 1 and
+    countParam = 2
   }
 
   override predicate hasArrayInput(int bufParam) { bufParam = 1 }
@@ -44,7 +47,9 @@ private class Send extends AliasFunction, ArrayFunction, SideEffectFunction, Rem
   override predicate hasSpecificReadSideEffect(ParameterIndex i, boolean buffer) {
     i = 1 and buffer = true
     or
-    exists(this.getParameter(4)) and i = 4 and buffer = false
+    this.hasGlobalName("sendto") and i = 4 and buffer = false
+    or
+    this.hasGlobalName("sendmsg") and i = 1 and buffer = true
   }
 
   override ParameterIndex getParameterSizeIndex(ParameterIndex i) { i = 1 and result = 2 }

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

@@ -1,9 +1,9 @@
 /**
- * Provides a class for modeling functions that return data from potentially untrusted sources. To use
- * this QL library, create a QL class extending `DataFlowFunction` with a
+ * Provides classes for modeling functions that return data from (or send data to) potentially untrusted
+ * sources. To use this QL library, create a QL class extending `DataFlowFunction` with a
  * characteristic predicate that selects the function or set of functions you
  * are modeling. Within that class, override the predicates provided by
- * `RemoteFlowFunction` to match the flow within that function.
+ * `RemoteFlowSourceFunction` or `RemoteFlowSinkFunction` to match the flow within that function.
  */
 
 import cpp
@@ -13,7 +13,7 @@ import semmle.code.cpp.models.Models
 /**
  * A library function that returns data that may be read from a network connection.
  */
-abstract class RemoteFlowFunction extends Function {
+abstract class RemoteFlowSourceFunction extends Function {
   /**
    * Holds if remote data described by `description` flows from `output` of a call to this function.
    */
@@ -23,15 +23,15 @@ abstract class RemoteFlowFunction extends Function {
 /**
  * A library function that returns data that is directly controlled by a user.
  */
-abstract class LocalFlowFunction extends Function {
+abstract class LocalFlowSourceFunction extends Function {
   /**
    * Holds if data described by `description` flows from `output` of a call to this function.
    */
   abstract predicate hasLocalFlowSource(FunctionOutput output, string description);
 }
 
 /** A library function that sends data over a network connection. */
-abstract class RemoteFlowFunctionSink extends Function {
+abstract class RemoteFlowSinkFunction extends Function {
   /**
    * Holds if data described by `description` flows into `input` to a call to this function, and is then
    * send over a network connection.

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

@@ -23,7 +23,7 @@ private class RemoteReturnSource extends RemoteFlowSource {
   string sourceType;
 
   RemoteReturnSource() {
-    exists(RemoteFlowFunction func, CallInstruction instr, FunctionOutput output |
+    exists(RemoteFlowSourceFunction func, CallInstruction instr, FunctionOutput output |
       asInstruction() = instr and
       instr.getStaticCallTarget() = func and
       func.hasRemoteFlowSource(output, sourceType) and
@@ -42,7 +42,7 @@ private class RemoteParameterSource extends RemoteFlowSource {
   string sourceType;
 
   RemoteParameterSource() {
-    exists(RemoteFlowFunction func, WriteSideEffectInstruction instr, FunctionOutput output |
+    exists(RemoteFlowSourceFunction func, WriteSideEffectInstruction instr, FunctionOutput output |
       asInstruction() = instr and
       instr.getPrimaryInstruction().(CallInstruction).getStaticCallTarget() = func and
       func.hasRemoteFlowSource(output, sourceType) and
@@ -57,7 +57,7 @@ private class LocalReturnSource extends LocalFlowSource {
   string sourceType;
 
   LocalReturnSource() {
-    exists(LocalFlowFunction func, CallInstruction instr, FunctionOutput output |
+    exists(LocalFlowSourceFunction func, CallInstruction instr, FunctionOutput output |
       asInstruction() = instr and
       instr.getStaticCallTarget() = func and
       func.hasLocalFlowSource(output, sourceType) and
@@ -76,7 +76,7 @@ private class LocalParameterSource extends LocalFlowSource {
   string sourceType;
 
   LocalParameterSource() {
-    exists(LocalFlowFunction func, WriteSideEffectInstruction instr, FunctionOutput output |
+    exists(LocalFlowSourceFunction func, WriteSideEffectInstruction instr, FunctionOutput output |
       asInstruction() = instr and
       instr.getPrimaryInstruction().(CallInstruction).getStaticCallTarget() = func and
       func.hasLocalFlowSource(output, sourceType) and
@@ -109,7 +109,7 @@ private class RemoteParameterSink extends RemoteFlowSink {
   string sourceType;
 
   RemoteParameterSink() {
-    exists(RemoteFlowFunctionSink func, FunctionInput input, CallInstruction call, int index |
+    exists(RemoteFlowSinkFunction func, FunctionInput input, CallInstruction call, int index |
       func.hasRemoteFlowSink(input, sourceType) and call.getStaticCallTarget() = func
     |
       exists(ReadSideEffectInstruction read |

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

@@ -60,17 +60,11 @@ class SecurityOptions extends string {
       or
       functionCall.getTarget().hasGlobalName(fname) and
       exists(functionCall.getArgument(arg)) and
-      (
-        fname = "recvmsg" and arg = 1
-        or
-        fname = "getaddrinfo" and arg = 3
-        or
-        fname = "recvfrom" and
-        (arg = 1 or arg = 4 or arg = 5)
-      )
+      fname = "getaddrinfo" and
+      arg = 3
     )
     or
-    exists(RemoteFlowFunction remote, FunctionOutput output |
+    exists(RemoteFlowSourceFunction remote, FunctionOutput output |
       functionCall.getTarget() = remote and
       output.isParameterDerefOrQualifierObject(arg) and
       remote.hasRemoteFlowSource(output, _)
@@ -89,7 +83,7 @@ class SecurityOptions extends string {
       )
     )
     or
-    exists(RemoteFlowFunction remote, FunctionOutput output |
+    exists(RemoteFlowSourceFunction remote, FunctionOutput output |
       functionCall.getTarget() = remote and
       (output.isReturnValue() or output.isReturnValueDeref()) and
       remote.hasRemoteFlowSource(output, _)