Merge pull request #1562 from geoffw0/models

CPP: Extend StrcpyFunction and update UsingStrcpyAsBoolean.ql

Author: Robert Marsh

cpp/ql/src/Likely Bugs/Likely Typos/UsingStrcpyAsBoolean.ql

@@ -12,20 +12,9 @@
  */
 
 import cpp
+import semmle.code.cpp.models.implementations.Strcpy
 import semmle.code.cpp.dataflow.DataFlow
 
-predicate isStringComparisonFunction(string functionName) {
-  functionName = "strcpy" or
-  functionName = "wcscpy" or
-  functionName = "_mbscpy" or
-  functionName = "strncpy" or
-  functionName = "_strncpy_l" or
-  functionName = "wcsncpy" or
-  functionName = "_wcsncpy_l" or
-  functionName = "_mbsncpy" or
-  functionName = "_mbsncpy_l"
-}
-
 predicate isBoolean(Expr e1) {
   exists(Type t1 |
     t1 = e1.getType() and
@@ -36,12 +25,12 @@ predicate isBoolean(Expr e1) {
 predicate isStringCopyCastedAsBoolean(FunctionCall func, Expr expr1, string msg) {
   DataFlow::localFlow(DataFlow::exprNode(func), DataFlow::exprNode(expr1)) and
   isBoolean(expr1.getConversion*()) and
-  isStringComparisonFunction(func.getTarget().getName()) and
+  func.getTarget() instanceof StrcpyFunction and
   msg = "Return value of " + func.getTarget().getName() + " used as a Boolean."
 }
 
 predicate isStringCopyUsedInLogicalOperationOrCondition(FunctionCall func, Expr expr1, string msg) {
-  isStringComparisonFunction(func.getTarget().getName()) and
+  func.getTarget() instanceof StrcpyFunction and
   (
     (
       // it is being used in an equality or logical operation

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

@@ -4,16 +4,19 @@ import semmle.code.cpp.models.interfaces.Taint
 
 
 /**
- * The standard function `stract` and its wide, sized, and Microsoft variants.
+ * The standard function `strcpy` and its wide, sized, and Microsoft variants.
  */
 class StrcpyFunction extends ArrayFunction, DataFlowFunction, TaintFunction {
   StrcpyFunction() {
     this.hasName("strcpy") or
     this.hasName("_mbscpy") or
     this.hasName("wcscpy") or
     this.hasName("strncpy") or
+    this.hasName("_strncpy_l") or
     this.hasName("_mbsncpy") or
-    this.hasName("wcsncpy")
+    this.hasName("_mbsncpy_l") or
+    this.hasName("wcsncpy") or
+    this.hasName("_wcsncpy_l")
   }
 
   override predicate hasArrayInput(int bufParam) {
@@ -31,13 +34,16 @@ class StrcpyFunction extends ArrayFunction, DataFlowFunction, TaintFunction {
   override predicate hasArrayWithVariableSize(int bufParam, int countParam) {
     (
       this.hasName("strncpy") or
+      this.hasName("_strncpy_l") or
       this.hasName("_mbsncpy") or
-      this.hasName("wcsncpy")
+      this.hasName("_mbsncpy_l") or
+      this.hasName("wcsncpy") or
+      this.hasName("_wcsncpy_l")
     ) and
     bufParam = 0 and
     countParam = 2
   }
-  
+
   override predicate hasArrayWithUnknownSize(int bufParam) {
     (
       this.hasName("strcpy") or
@@ -46,7 +52,6 @@ class StrcpyFunction extends ArrayFunction, DataFlowFunction, TaintFunction {
     ) and
     bufParam = 0
   }
-  
 
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
     (
@@ -70,14 +75,17 @@ class StrcpyFunction extends ArrayFunction, DataFlowFunction, TaintFunction {
       output.isOutReturnValue()
     )
   }
-  
+
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     (
       // these may do only a partial copy of the input buffer to the output
       // buffer
       this.hasName("strncpy") or
+      this.hasName("_strncpy_l") or
       this.hasName("_mbsncpy") or
-      this.hasName("wcsncpy")
+      this.hasName("_mbsncpy_l") or
+      this.hasName("wcsncpy") or
+      this.hasName("_wcsncpy_l")
     ) and (
       input.isInParameter(2) or
       input.isInParameterPointer(1)