C++: Fix flow for return values of strlcat and strlcpy

Author: jketema

cpp/ql/lib/semmle/code/cpp/models/implementations/Strcat.qll

@@ -96,7 +96,7 @@ class StrcatFunction extends TaintFunction, DataFlowFunction, ArrayFunction, Sid
 /**
  * The `strlcat` function.
  */
-class StrlcatFunction extends TaintFunction, DataFlowFunction, ArrayFunction, SideEffectFunction {
+class StrlcatFunction extends TaintFunction, ArrayFunction, SideEffectFunction {
   StrlcatFunction() {
     this.hasGlobalName("strlcat") // strlcat(dst, src, dst_size)
   }
@@ -116,11 +116,6 @@ class StrlcatFunction extends TaintFunction, DataFlowFunction, ArrayFunction, Si
    */
   int getParamDest() { result = 0 }
 
-  override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
-    input.isParameter(0) and
-    output.isReturnValue()
-  }
-
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     (
       input.isParameter(2)
@@ -129,7 +124,7 @@ class StrlcatFunction extends TaintFunction, DataFlowFunction, ArrayFunction, Si
       or
       input.isParameterDeref(1)
     ) and
-    (output.isParameterDeref(0) or output.isReturnValueDeref())
+    (output.isParameterDeref(0) or output.isReturnValue())
   }
 
   override predicate hasArrayInput(int param) {

cpp/ql/lib/semmle/code/cpp/models/implementations/Strcpy.qll

@@ -54,14 +54,19 @@ class StrcpyFunction extends ArrayFunction, DataFlowFunction, TaintFunction, Sid
    */
   private predicate isSVariant() { this.getName().matches("%\\_s") }
 
+  /**
+   * Holds if the function returns the total length the string would have had if the size was unlimited.
+   */
+  private predicate returnsTotalLength() { this.getName() = "strlcpy" }
+
   /**
    * Gets the index of the parameter that is the maximum size of the copy (in characters).
    */
   int getParamSize() {
     if this.isSVariant()
     then result = 1
     else (
-      this.getName().matches(["%ncpy%", "%nbcpy%", "%xfrm%", "%lcpy%"]) and
+      this.getName().matches(["%ncpy%", "%nbcpy%", "%xfrm%", "strlcpy"]) and
       result = 2
     )
   }
@@ -101,6 +106,7 @@ class StrcpyFunction extends ArrayFunction, DataFlowFunction, TaintFunction, Sid
     input.isParameterDeref(this.getParamSrc()) and
     output.isReturnValueDeref()
     or
+    not this.returnsTotalLength() and
     input.isParameter(this.getParamDest()) and
     output.isReturnValue()
   }
@@ -111,8 +117,9 @@ class StrcpyFunction extends ArrayFunction, DataFlowFunction, TaintFunction, Sid
     exists(this.getParamSize()) and
     input.isParameterDeref(this.getParamSrc()) and
     (
-      output.isParameterDeref(this.getParamDest()) or
-      output.isReturnValueDeref()
+      output.isParameterDeref(this.getParamDest())
+      or
+      not this.returnsTotalLength() and output.isReturnValueDeref()
     )
   }