Merge pull request github#5046 from MathiasVP/model-more-pure-functions

C++: Model more pure functions

Author: geoffw0

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

@@ -8,9 +8,8 @@ private class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunctio
   SideEffectFunction {
   PureStrFunction() {
     hasGlobalOrStdName([
-        "atof", "atoi", "atol", "atoll", "strcasestr", "strchnul", "strchr", "strchrnul", "strstr",
-        "strpbrk", "strcmp", "strcspn", "strncmp", "strrchr", "strspn", "strtod", "strtof",
-        "strtol", "strtoll", "strtoq", "strtoul"
+        atoi(), "strcasestr", "strchnul", "strchr", "strchrnul", "strstr", "strpbrk", "strrchr",
+        "strspn", strtol(), strrev(), strcmp(), strlwr(), strupr()
       ])
   }
 
@@ -24,11 +23,16 @@ private class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunctio
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     exists(ParameterIndex i |
-      input.isParameter(i) and
-      exists(getParameter(i))
-      or
-      input.isParameterDeref(i) and
-      getParameter(i).getUnspecifiedType() instanceof PointerType
+      (
+        input.isParameter(i) and
+        exists(getParameter(i))
+        or
+        input.isParameterDeref(i) and
+        getParameter(i).getUnspecifiedType() instanceof PointerType
+      ) and
+      // Functions that end with _l also take a locale argument (always as the last argument),
+      // and we don't want taint from those arguments.
+      (not this.getName().matches("%\\_l") or exists(getParameter(i + 1)))
     ) and
     (
       output.isReturnValueDeref() and
@@ -60,6 +64,31 @@ private class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunctio
   }
 }
 
+private string atoi() { result = ["atof", "atoi", "atol", "atoll"] }
+
+private string strtol() { result = ["strtod", "strtof", "strtol", "strtoll", "strtoq", "strtoul"] }
+
+private string strlwr() {
+  result = ["_strlwr", "_wcslwr", "_mbslwr", "_strlwr_l", "_wcslwr_l", "_mbslwr_l"]
+}
+
+private string strupr() {
+  result = ["_strupr", "_wcsupr", "_mbsupr", "_strupr_l", "_wcsupr_l", "_mbsupr_l"]
+}
+
+private string strrev() { result = ["_strrev", "_wcsrev", "_mbsrev", "_mbsrev_l"] }
+
+private string strcmp() {
+  // NOTE: `strcoll` doesn't satisfy _all_ the definitions of purity: its behavior depends on
+  // `LC_COLLATE` (which is set by `setlocale`). Not sure this behavior worth including in the model, so
+  // for now we interpret the function as being pure.
+  result =
+    [
+      "strcmp", "strcspn", "strncmp", "strcoll", "strverscmp", "_mbsnbcmp", "_mbsnbcmp_l",
+      "_stricmp"
+    ]
+}
+
 /** String standard `strlen` function, and related functions for computing string lengths. */
 private class StrLenFunction extends AliasFunction, ArrayFunction, SideEffectFunction {
   StrLenFunction() {
@@ -114,19 +143,28 @@ private class PureFunction extends TaintFunction, SideEffectFunction {
 /** Pure raw-memory functions. */
 private class PureMemFunction extends AliasFunction, ArrayFunction, TaintFunction,
   SideEffectFunction {
-  PureMemFunction() { hasGlobalOrStdName(["memchr", "memrchr", "rawmemchr", "memcmp", "memmem"]) }
+  PureMemFunction() {
+    hasGlobalOrStdName([
+        "memchr", "__builtin_memchr", "memrchr", "rawmemchr", "memcmp", "__builtin_memcmp", "memmem"
+      ]) or
+    this.hasGlobalName("memfrob")
+  }
 
   override predicate hasArrayInput(int bufParam) {
     getParameter(bufParam).getUnspecifiedType() instanceof PointerType
   }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     exists(ParameterIndex i |
-      input.isParameter(i) and
-      exists(getParameter(i))
-      or
-      input.isParameterDeref(i) and
-      getParameter(i).getUnspecifiedType() instanceof PointerType
+      (
+        input.isParameter(i) and
+        exists(getParameter(i))
+        or
+        input.isParameterDeref(i) and
+        getParameter(i).getUnspecifiedType() instanceof PointerType
+      ) and
+      // `memfrob` should not have taint from the size argument.
+      (not this.hasGlobalName("memfrob") or i = 0)
     ) and
     (
       output.isReturnValueDeref() and