Updated getResultExpr to getResultNode. Added strlcat. Added tests.

Author: bdrodes

cpp/ql/lib/semmle/code/cpp/commons/StringConcatenation.qll

@@ -5,14 +5,16 @@
 import cpp
 import semmle.code.cpp.models.implementations.Strcat
 import semmle.code.cpp.models.interfaces.FormattingFunction
+import semmle.code.cpp.dataflow.new.DataFlow
 
 class StringConcatenation extends Call {
   StringConcatenation() {
-    // printf-like functions, i.e., concat through formating
+    // sprintf-like functions, i.e., concat through formating
     exists(FormattingFunctionCall fc | this = fc)
     or
-    // strcat variants
-    exists(StrcatFunction f | this.getTarget() = f)
+    this.getTarget() instanceof StrcatFunction
+    or
+    this.getTarget() instanceof StrlcatFunction
     or
     // operator+ concat
     exists(Call call, Operator op |
@@ -35,7 +37,9 @@ class StringConcatenation extends Call {
   Expr getAnOperand() {
     // The result is an argument of 'this' (a call)
     result = this.getAnArgument() and
-    not result instanceof Call and // addresses odd behavior with overloaded operators
+    // addresses odd behavior with overloaded operators
+    // i.e., "call to operator+" appearing as an operand
+    not result instanceof Call and
     // Limit the result type to string
     (
       result.getUnderlyingType().stripType().getName() = "char"
@@ -69,11 +73,26 @@ class StringConcatenation extends Call {
   }
 
   /**
-   * Gets the expression representing the concatenation result.
+   * Gets the data flow node representing the concatenation result.
    */
-  Expr getResultExpr() {
-    if this instanceof FormattingFunctionCall
-    then result = this.(FormattingFunctionCall).getOutputArgument(_)
-    else result = this.(Call)
+  DataFlow::Node getResultNode() {
+    if this.getTarget() instanceof StrcatFunction
+    then
+      result.asDefiningArgument() =
+        this.getArgument(this.getTarget().(StrcatFunction).getParamDest())
+      or
+      // Hardcoding it is also the return
+      [result.asExpr(), result.asIndirectExpr()] = this.(Call)
+    else
+      if this.getTarget() instanceof StrlcatFunction
+      then (
+        [result.asExpr(), result.asIndirectExpr()] =
+          this.getArgument(this.getTarget().(StrlcatFunction).getParamDest())
+      ) else
+        if this instanceof FormattingFunctionCall
+        then
+          [result.asExpr(), result.asIndirectExpr()] =
+            this.(FormattingFunctionCall).getOutputArgument(_)
+        else [result.asExpr(), result.asIndirectExpr()] = this.(Call)
   }
 }

cpp/ql/test/library-tests/string_concat/concat.cpp

@@ -0,0 +1,62 @@
+// #include <iostream>
+// #include <string>
+// #include <stdio.h>
+// #include <string.h>
+// #include <sstream>
+#include "stl.h"
+
+int sprintf(char *s, const char *format, ...);
+char *strcat(char * s1, const char * s2);
+
+using namespace std;
+
+
+void test1(){
+    string str1 = "Hello";
+    string str2 = "World";
+    string str3 = "!";
+    string str4 = "Concatenation";
+    string str5 = "is";
+    string str6 = "fun";
+
+    // Using the + operator
+    string result1 = str1 + " " + str2 + str3;
+
+    // Using the append() function
+    //----TODO: currently not modeled----
+    // string result2 = str4.append(" ") + str5.append(" ") + str6;
+
+    // Using the insert() function
+    //----TODO: currently not modeled----
+    // string result3 = str1.insert(5, " ") + str2.insert(5, "! ");
+
+    // Using the replace() function
+    //----TODO: currently not modeled----
+    // string result4 = str1.replace(0, 5, "Hi") + str2.replace(0, 5, "There");
+
+    // Using the push_back() function
+    //----TODO: currently not modeled----
+    // string result5;
+    // for (char c : str1) {
+    //     result5.push_back(c);
+    // }
+
+    // Using the stream operator
+    string result6;
+    std::stringstream ss;
+    ss << str1 << " " << str2 << str3;
+}
+
+
+void test2(char* ucstr) {
+    char str1[20] = "Hello";
+    char str2[20] = "World";
+    char result[40];
+    char *result2;
+
+    // Using sprintf
+    sprintf(result, "%s %s %s", str1, str2, ucstr);
+
+    // Using strcat
+    strcat(str1, ucstr);
+}