Merge pull request github#1938 from dave-bartolomeo/dave/InNOut

C++: Rename predicates in `FunctionInputsAndOutputs.qll` and add QLDoc

Author: jbj

change-notes/1.23/analysis-cpp.md

@@ -38,3 +38,7 @@ The following changes in version 1.23 affect C/C++ analysis in all applications.
 * There is now a `DataFlow::localExprFlow` predicate and a
   `TaintTracking::localExprTaint` predicate to make it easy to use the most
   common case of local data flow and taint: from one `Expr` to another.
+* The member predicates of the `FunctionInput` and `FunctionOutput` classes have been renamed for
+  clarity (e.g. `isOutReturnPointer()` to `isReturnValueDeref()`). The existing member predicates
+  have been deprecated, and will be removed in a future release. Code that uses the old member
+  predicates should be updated to use the corresponding new member predicate.

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll

@@ -578,8 +578,8 @@ private predicate exprToExprStep_nocfg(Expr fromExpr, Expr toExpr) {
       exists(DataFlowFunction f, FunctionInput inModel, FunctionOutput outModel, int iIn |
         call.getTarget() = f and
         f.hasDataFlow(inModel, outModel) and
-        outModel.isOutReturnValue() and
-        inModel.isInParameter(iIn) and
+        outModel.isReturnValue() and
+        inModel.isParameter(iIn) and
         fromExpr = call.getArgument(iIn)
       )
     )
@@ -589,12 +589,12 @@ private predicate exprToDefinitionByReferenceStep(Expr exprIn, Expr argOut) {
   exists(DataFlowFunction f, Call call, FunctionOutput outModel, int argOutIndex |
     call.getTarget() = f and
     argOut = call.getArgument(argOutIndex) and
-    outModel.isOutParameterPointer(argOutIndex) and
+    outModel.isParameterDeref(argOutIndex) and
     exists(int argInIndex, FunctionInput inModel | f.hasDataFlow(inModel, outModel) |
-      inModel.isInParameterPointer(argInIndex) and
+      inModel.isParameterDeref(argInIndex) and
       call.passesByReference(argInIndex, exprIn)
       or
-      inModel.isInParameter(argInIndex) and
+      inModel.isParameter(argInIndex) and
       exprIn = call.getArgument(argInIndex)
     )
   )

cpp/ql/src/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll

@@ -122,27 +122,27 @@ private predicate exprToDefinitionByReferenceStep(Expr exprIn, Expr argOut) {
   exists(DataFlowFunction f, Call call, FunctionOutput outModel, int argOutIndex |
     call.getTarget() = f and
     argOut = call.getArgument(argOutIndex) and
-    outModel.isOutParameterPointer(argOutIndex) and
+    outModel.isParameterDeref(argOutIndex) and
     exists(int argInIndex, FunctionInput inModel | f.hasDataFlow(inModel, outModel) |
       // Taint flows from a pointer to a dereference, which DataFlow does not handle
       // memcpy(&dest_var, tainted_ptr, len)
-      inModel.isInParameterPointer(argInIndex) and
+      inModel.isParameterDeref(argInIndex) and
       exprIn = call.getArgument(argInIndex)
     )
   )
   or
   exists(TaintFunction f, Call call, FunctionOutput outModel, int argOutIndex |
     call.getTarget() = f and
     argOut = call.getArgument(argOutIndex) and
-    outModel.isOutParameterPointer(argOutIndex) and
+    outModel.isParameterDeref(argOutIndex) and
     exists(int argInIndex, FunctionInput inModel | f.hasTaintFlow(inModel, outModel) |
-      inModel.isInParameterPointer(argInIndex) and
+      inModel.isParameterDeref(argInIndex) and
       exprIn = call.getArgument(argInIndex)
       or
-      inModel.isInParameterPointer(argInIndex) and
+      inModel.isParameterDeref(argInIndex) and
       call.passesByReference(argInIndex, exprIn)
       or
-      inModel.isInParameter(argInIndex) and
+      inModel.isParameter(argInIndex) and
       exprIn = call.getArgument(argInIndex)
     )
   )

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

@@ -34,6 +34,6 @@ class IdentityFunction extends DataFlowFunction, SideEffectFunction, AliasFuncti
 
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
     // These functions simply return the argument value.
-    input.isInParameter(0) and output.isOutReturnValue()
+    input.isParameter(0) and output.isReturnValue()
   }
 }

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

@@ -5,17 +5,17 @@ class InetNtoa extends TaintFunction {
   InetNtoa() { hasGlobalName("inet_ntoa") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
-    input.isInParameter(0) and
-    output.isOutReturnPointer()
+    input.isParameter(0) and
+    output.isReturnValueDeref()
   }
 }
 
 class InetAton extends TaintFunction, ArrayFunction {
   InetAton() { hasGlobalName("inet_aton") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
-    input.isInParameterPointer(0) and
-    output.isOutParameterPointer(1)
+    input.isParameterDeref(0) and
+    output.isParameterDeref(1)
   }
 
   override predicate hasArrayInput(int bufParam) { bufParam = 0 }
@@ -34,8 +34,8 @@ class InetAddr extends TaintFunction, ArrayFunction {
   InetAddr() { hasGlobalName("inet_addr") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
-    input.isInParameterPointer(0) and
-    output.isOutReturnValue()
+    input.isParameterDeref(0) and
+    output.isReturnValue()
   }
 
   override predicate hasArrayInput(int bufParam) { bufParam = 0 }
@@ -47,8 +47,8 @@ class InetNetwork extends TaintFunction, ArrayFunction {
   InetNetwork() { hasGlobalName("inet_network") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
-    input.isInParameterPointer(1) and
-    output.isOutReturnValue()
+    input.isParameterDeref(1) and
+    output.isReturnValue()
   }
 
   override predicate hasArrayInput(int bufParam) { bufParam = 0 }
@@ -61,28 +61,28 @@ class InetMakeaddr extends TaintFunction {
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     (
-      input.isInParameter(0) or
-      input.isInParameter(1)
+      input.isParameter(0) or
+      input.isParameter(1)
     ) and
-    output.isOutReturnValue()
+    output.isReturnValue()
   }
 }
 
 class InetLnaof extends TaintFunction {
   InetLnaof() { hasGlobalName("inet_lnaof") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
-    input.isInParameter(0) and
-    output.isOutReturnValue()
+    input.isParameter(0) and
+    output.isReturnValue()
   }
 }
 
 class InetNetof extends TaintFunction {
   InetNetof() { hasGlobalName("inet_netof") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
-    input.isInParameter(0) and
-    output.isOutReturnValue()
+    input.isParameter(0) and
+    output.isReturnValue()
   }
 }
 
@@ -91,10 +91,10 @@ class InetPton extends TaintFunction, ArrayFunction {
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     (
-      input.isInParameter(0) or
-      input.isInParameterPointer(1)
+      input.isParameter(0) or
+      input.isParameterDeref(1)
     ) and
-    output.isOutParameterPointer(2)
+    output.isParameterDeref(2)
   }
 
   override predicate hasArrayInput(int bufParam) { bufParam = 1 }
@@ -110,8 +110,8 @@ class Gethostbyname extends TaintFunction, ArrayFunction {
   Gethostbyname() { hasGlobalName("gethostbyname") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
-    input.isInParameterPointer(0) and
-    output.isOutReturnPointer()
+    input.isParameterDeref(0) and
+    output.isReturnValueDeref()
   }
 
   override predicate hasArrayInput(int bufParam) { bufParam = 0 }
@@ -124,11 +124,11 @@ class Gethostbyaddr extends TaintFunction, ArrayFunction {
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     (
-      input.isInParameterPointer(0) or
-      input.isInParameter(1) or
-      input.isInParameter(2)
+      input.isParameterDeref(0) or
+      input.isParameter(1) or
+      input.isParameter(2)
     ) and
-    output.isOutReturnPointer()
+    output.isReturnValueDeref()
   }
 
   override predicate hasArrayInput(int bufParam) { bufParam = 0 }

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

@@ -19,22 +19,22 @@ class MemcpyFunction extends ArrayFunction, DataFlowFunction, TaintFunction {
   override predicate hasArrayOutput(int bufParam) { bufParam = 0 }
 
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
-    input.isInParameterPointer(1) and
-    output.isOutParameterPointer(0)
+    input.isParameterDeref(1) and
+    output.isParameterDeref(0)
     or
-    input.isInParameterPointer(1) and
-    output.isOutReturnPointer()
+    input.isParameterDeref(1) and
+    output.isReturnValueDeref()
     or
-    input.isInParameter(0) and
-    output.isOutReturnValue()
+    input.isParameter(0) and
+    output.isReturnValue()
   }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
-    input.isInParameter(2) and
-    output.isOutParameterPointer(0)
+    input.isParameter(2) and
+    output.isParameterDeref(0)
     or
-    input.isInParameter(2) and
-    output.isOutReturnPointer()
+    input.isParameter(2) and
+    output.isReturnValueDeref()
   }
 
   override predicate hasArrayWithVariableSize(int bufParam, int countParam) {

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

@@ -41,17 +41,17 @@ class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunction, SideE
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     exists(ParameterIndex i |
-      input.isInParameter(i) and
+      input.isParameter(i) and
       exists(getParameter(i))
       or
-      input.isInParameterPointer(i) and
+      input.isParameterDeref(i) and
       getParameter(i).getUnspecifiedType() instanceof PointerType
     ) and
     (
-      output.isOutReturnPointer() and
+      output.isReturnValueDeref() and
       getUnspecifiedType() instanceof PointerType
       or
-      output.isOutReturnValue()
+      output.isReturnValue()
     )
   }
 
@@ -85,10 +85,10 @@ class PureFunction extends TaintFunction, SideEffectFunction {
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     exists(ParameterIndex i |
-      input.isInParameter(i) and
+      input.isParameter(i) and
       exists(getParameter(i))
     ) and
-    output.isOutReturnValue()
+    output.isReturnValue()
   }
 
   override predicate neverReadsMemory() { any() }

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

@@ -19,8 +19,8 @@ class StrcatFunction extends TaintFunction, DataFlowFunction, ArrayFunction {
   }
 
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
-    input.isInParameter(0) and
-    output.isOutReturnValue()
+    input.isParameter(0) and
+    output.isReturnValue()
   }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -31,19 +31,19 @@ class StrcatFunction extends TaintFunction, DataFlowFunction, ArrayFunction {
         name = "_mbsncat" or
         name = "_mbsncat_l"
       ) and
-      input.isInParameter(2) and
-      output.isOutParameterPointer(0)
+      input.isParameter(2) and
+      output.isParameterDeref(0)
       or
       name = "_mbsncat_l" and
-      input.isInParameter(3) and
-      output.isOutParameterPointer(0)
+      input.isParameter(3) and
+      output.isParameterDeref(0)
     )
     or
-    input.isInParameterPointer(0) and
-    output.isOutParameterPointer(0)
+    input.isParameterDeref(0) and
+    output.isParameterDeref(0)
     or
-    input.isInParameter(1) and
-    output.isOutParameterPointer(0)
+    input.isParameter(1) and
+    output.isParameterDeref(0)
   }
 
   override predicate hasArrayInput(int param) {

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

@@ -55,15 +55,15 @@ class StrcpyFunction extends ArrayFunction, DataFlowFunction, TaintFunction {
       this.hasName("wcscpy")
     ) and
     (
-      input.isInParameterPointer(1) and
-      output.isOutParameterPointer(0)
+      input.isParameterDeref(1) and
+      output.isParameterDeref(0)
       or
-      input.isInParameterPointer(1) and
-      output.isOutReturnPointer()
+      input.isParameterDeref(1) and
+      output.isReturnValueDeref()
     )
     or
-    input.isInParameter(0) and
-    output.isOutReturnValue()
+    input.isParameter(0) and
+    output.isReturnValue()
   }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -78,12 +78,12 @@ class StrcpyFunction extends ArrayFunction, DataFlowFunction, TaintFunction {
       this.hasName("_wcsncpy_l")
     ) and
     (
-      input.isInParameter(2) or
-      input.isInParameterPointer(1)
+      input.isParameter(2) or
+      input.isParameterDeref(1)
     ) and
     (
-      output.isOutParameterPointer(0) or
-      output.isOutReturnPointer()
+      output.isParameterDeref(0) or
+      output.isReturnValueDeref()
     )
   }
 }

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

@@ -6,13 +6,13 @@ class Strftime extends TaintFunction, ArrayFunction {
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     (
-      input.isInParameter(1) or
-      input.isInParameterPointer(2) or
-      input.isInParameterPointer(3)
+      input.isParameter(1) or
+      input.isParameterDeref(2) or
+      input.isParameterDeref(3)
     ) and
     (
-      output.isOutParameterPointer(0) or
-      output.isOutReturnValue()
+      output.isParameterDeref(0) or
+      output.isReturnValue()
     )
   }