Merge pull request #637 from knewbury01/knewbury01/fix-119

M0-2-1: make into split and shared query

Author: knewbury01

c/cert/src/rules/ARR39-C/DoNotAddOrSubtractAScaledIntegerToAPointer.ql

@@ -13,7 +13,7 @@
 
 import cpp
 import codingstandards.c.cert
-import codingstandards.c.Pointers
+import codingstandards.cpp.Pointers
 import codingstandards.cpp.dataflow.TaintTracking
 import ScaledIntegerPointerArithmeticFlow::PathGraph
 

c/cert/src/rules/EXP43-C/DoNotPassAliasedPointerToRestrictQualifiedParam.ql

@@ -12,177 +12,11 @@
 
 import cpp
 import codingstandards.c.cert
-import codingstandards.c.Pointers
-import codingstandards.c.Variable
-import codingstandards.cpp.dataflow.DataFlow
-import semmle.code.cpp.pointsto.PointsTo
-import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
+import codingstandards.cpp.rules.donotpassaliasedpointertorestrictqualifiedparamshared.DoNotPassAliasedPointerToRestrictQualifiedParamShared
 
-/**
- * A function that has a parameter with a restrict-qualified pointer type.
- */
-class FunctionWithRestrictParameters extends Function {
-  Parameter restrictPtrParam;
-
-  FunctionWithRestrictParameters() {
-    restrictPtrParam.getUnspecifiedType() instanceof PointerOrArrayType and
-    (
-      restrictPtrParam.getType().hasSpecifier(["restrict"]) and
-      restrictPtrParam = this.getAParameter()
-      or
-      this.hasGlobalName(["strcpy", "strncpy", "strcat", "strncat", "memcpy"]) and
-      restrictPtrParam = this.getParameter([0, 1])
-      or
-      this.hasGlobalName(["strcpy_s", "strncpy_s", "strcat_s", "strncat_s", "memcpy_s"]) and
-      restrictPtrParam = this.getParameter([0, 2])
-      or
-      this.hasGlobalName(["strtok_s"]) and
-      restrictPtrParam = this.getAParameter()
-      or
-      this.hasGlobalName(["printf", "printf_s", "scanf", "scanf_s"]) and
-      restrictPtrParam = this.getParameter(0)
-      or
-      this.hasGlobalName(["sprintf", "sprintf_s", "snprintf", "snprintf_s"]) and
-      restrictPtrParam = this.getParameter(3)
-    )
-  }
-
-  Parameter getARestrictPtrParam() { result = restrictPtrParam }
-}
-
-/**
- * A call to a function that has a parameter with a restrict-qualified pointer type.
- */
-class CallToFunctionWithRestrictParameters extends FunctionCall {
-  CallToFunctionWithRestrictParameters() {
-    this.getTarget() instanceof FunctionWithRestrictParameters
-  }
-
-  Expr getARestrictPtrArg() {
-    result =
-      this.getArgument(this.getTarget()
-            .(FunctionWithRestrictParameters)
-            .getARestrictPtrParam()
-            .getIndex())
-  }
-
-  Expr getAPtrArg(int index) {
-    result = this.getArgument(index) and
-    pointerValue(result)
-  }
-
-  Expr getAPossibleSizeArg() {
-    exists(Parameter param |
-      param = this.getTarget().(FunctionWithRestrictParameters).getAParameter() and
-      param.getUnderlyingType() instanceof IntegralType and
-      // exclude __builtin_object_size
-      not result.(FunctionCall).getTarget() instanceof BuiltInFunction and
-      result = this.getArgument(param.getIndex())
-    )
-  }
-}
-
-/**
- * A `PointsToExpr` that is an argument of a pointer-type in a `CallToFunctionWithRestrictParameters`
- */
-class CallToFunctionWithRestrictParametersArgExpr extends Expr {
-  int paramIndex;
-
-  CallToFunctionWithRestrictParametersArgExpr() {
-    this = any(CallToFunctionWithRestrictParameters call).getAPtrArg(paramIndex)
+class DoNotPassAliasedPointerToRestrictQualifiedParamQuery extends DoNotPassAliasedPointerToRestrictQualifiedParamSharedSharedQuery
+{
+  DoNotPassAliasedPointerToRestrictQualifiedParamQuery() {
+    this = Pointers3Package::doNotPassAliasedPointerToRestrictQualifiedParamQuery()
   }
-
-  int getParamIndex() { result = paramIndex }
-}
-
-int getStatedValue(Expr e) {
-  // `upperBound(e)` defaults to `exprMaxVal(e)` when `e` isn't analyzable. So to get a meaningful
-  // result in this case we pick the minimum value obtainable from dataflow and range analysis.
-  result =
-    upperBound(e)
-        .minimum(min(Expr source | DataFlow::localExprFlow(source, e) | source.getValue().toInt()))
-}
-
-int getPointerArithmeticOperandStatedValue(CallToFunctionWithRestrictParametersArgExpr expr) {
-  result = getStatedValue(expr.(PointerArithmeticExpr).getOperand())
-  or
-  // edge-case: &(array[index]) expressions
-  result = getStatedValue(expr.(AddressOfExpr).getOperand().(PointerArithmeticExpr).getOperand())
-  or
-  // fall-back if `expr` is not a pointer arithmetic expression
-  not expr instanceof PointerArithmeticExpr and
-  not expr.(AddressOfExpr).getOperand() instanceof PointerArithmeticExpr and
-  result = 0
 }
-
-module PointerValueToRestrictArgConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node source) { pointerValue(source.asExpr()) }
-
-  predicate isSink(DataFlow::Node sink) {
-    exists(CallToFunctionWithRestrictParameters call |
-      sink.asExpr() = call.getAPtrArg(_).getAChild*()
-    )
-  }
-
-  predicate isBarrierIn(DataFlow::Node node) {
-    exists(AddressOfExpr a | node.asExpr() = a.getOperand().getAChild*())
-  }
-}
-
-module PointerValueToRestrictArgFlow = DataFlow::Global<PointerValueToRestrictArgConfig>;
-
-from
-  CallToFunctionWithRestrictParameters call, CallToFunctionWithRestrictParametersArgExpr arg1,
-  CallToFunctionWithRestrictParametersArgExpr arg2, int argOffset1, int argOffset2, Expr source1,
-  Expr source2, string sourceMessage1, string sourceMessage2
-where
-  not isExcluded(call, Pointers3Package::doNotPassAliasedPointerToRestrictQualifiedParamQuery()) and
-  arg1 = call.getARestrictPtrArg() and
-  arg2 = call.getAPtrArg(_) and
-  // enforce ordering to remove permutations if multiple restrict-qualified args exist
-  (not arg2 = call.getARestrictPtrArg() or arg2.getParamIndex() > arg1.getParamIndex()) and
-  (
-    // check if two pointers address the same object
-    PointerValueToRestrictArgFlow::flow(DataFlow::exprNode(source1),
-      DataFlow::exprNode(arg1.getAChild*())) and
-    (
-      // one pointer value flows to both args
-      PointerValueToRestrictArgFlow::flow(DataFlow::exprNode(source1),
-        DataFlow::exprNode(arg2.getAChild*())) and
-      sourceMessage1 = "$@" and
-      sourceMessage2 = "source" and
-      source1 = source2
-      or
-      // there are two separate values that flow from an AddressOfExpr of the same target
-      getAddressOfExprTargetBase(source1) = getAddressOfExprTargetBase(source2) and
-      PointerValueToRestrictArgFlow::flow(DataFlow::exprNode(source2),
-        DataFlow::exprNode(arg2.getAChild*())) and
-      sourceMessage1 = "a pair of address-of expressions ($@, $@)" and
-      sourceMessage2 = "addressof1" and
-      not source1 = source2
-    )
-  ) and
-  // get the offset of the pointer arithmetic operand (or '0' if there is none)
-  argOffset1 = getPointerArithmeticOperandStatedValue(arg1) and
-  argOffset2 = getPointerArithmeticOperandStatedValue(arg2) and
-  (
-    // case 1: the pointer args are the same.
-    // (definite aliasing)
-    argOffset1 = argOffset2
-    or
-    // case 2: the pointer args are different, a size arg exists,
-    // and the size arg is greater than the difference between the offsets.
-    // (potential aliasing)
-    exists(Expr sizeArg |
-      sizeArg = call.getAPossibleSizeArg() and
-      getStatedValue(sizeArg) > (argOffset1 - argOffset2).abs()
-    )
-    or
-    // case 3: the pointer args are different, and a size arg does not exist
-    // (potential aliasing)
-    not exists(call.getAPossibleSizeArg())
-  )
-select call,
-  "Call to '" + call.getTarget().getName() + "' passes an $@ to a $@ (pointer value derived from " +
-    sourceMessage1 + ".", arg2, "aliased pointer", arg1, "restrict-qualified parameter", source1,
-  sourceMessage2, source2, "addressof2"

c/cert/src/rules/EXP43-C/RestrictPointerReferencesOverlappingObject.ql

@@ -14,7 +14,7 @@ import cpp
 import codingstandards.cpp.dataflow.DataFlow
 import semmle.code.cpp.controlflow.Dominance
 import codingstandards.c.cert
-import codingstandards.c.Variable
+import codingstandards.cpp.Variable
 
 /**
  * An `Expr` that is an assignment or initialization to a restrict-qualified pointer-type variable.

c/cert/test/rules/EXP43-C/DoNotPassAliasedPointerToRestrictQualifiedParam.qlref

@@ -0,0 +1 @@
+c/common/test/rules/donotpassaliasedpointertorestrictqualifiedparamshared/DoNotPassAliasedPointerToRestrictQualifiedParamShared.ql

c/cert/test/rules/EXP43-C/DoNotPassAliasedPointerToRestrictQualifiedParam.testref

@@ -5,7 +5,7 @@
  */
 
 import cpp
-import codingstandards.c.Pointers
+import codingstandards.cpp.Pointers
 import codingstandards.c.Variable
 import codingstandards.cpp.Allocations
 import codingstandards.cpp.Overflow

c/common/src/codingstandards/c/OutOfBounds.qll

@@ -39,20 +39,6 @@ class FlexibleArrayMemberCandidate extends MemberVariable {
   }
 }
 
-/**
- * Returns the target variable of a `VariableAccess`.
- * If the access is a field access, then the target is the `Variable` of the qualifier.
- * If the access is an array access, then the target is the array base.
- */
-Variable getAddressOfExprTargetBase(AddressOfExpr expr) {
-  result = expr.getOperand().(ValueFieldAccess).getQualifier().(VariableAccess).getTarget()
-  or
-  not expr.getOperand() instanceof ValueFieldAccess and
-  result = expr.getOperand().(VariableAccess).getTarget()
-  or
-  result = expr.getOperand().(ArrayExpr).getArrayBase().(VariableAccess).getTarget()
-}
-
 /**
  * A struct that contains a flexible array member
  */

c/common/src/codingstandards/c/Variable.qll

@@ -0,0 +1,6 @@
+// GENERATED FILE - DO NOT MODIFY
+import codingstandards.cpp.rules.donotpassaliasedpointertorestrictqualifiedparamshared.DoNotPassAliasedPointerToRestrictQualifiedParamShared
+
+class TestFileQuery extends DoNotPassAliasedPointerToRestrictQualifiedParamSharedSharedQuery,
+  TestQuery
+{ }

c/cert/test/rules/EXP43-C/DoNotPassAliasedPointerToRestrictQualifiedParam.expected renamed to c/common/test/rules/donotpassaliasedpointertorestrictqualifiedparamshared/DoNotPassAliasedPointerToRestrictQualifiedParamShared.expected

@@ -0,0 +1,100 @@
+#include <stddef.h>
+#include <stdio.h>
+#include <string.h>
+
+int *restrict g1;
+int *restrict g2;
+int *restrict g1_1;
+int *g2_1;
+
+struct s1 {
+  int x, y, z;
+};
+struct s1 v1;
+
+void test_global_local() {
+  int *restrict i1 = g1; // COMPLIANT
+  int *restrict i2 = g2; // COMPLIANT
+  int *restrict i3 = i2; // NON_COMPLIANT
+  g1 = g2;               // NON_COMPLIANT
+  i1 = i2;               // NON_COMPLIANT
+  {
+    int *restrict i4;
+    int *restrict i5;
+    int *restrict i6;
+    i4 = g1;        // COMPLIANT
+    i4 = (void *)0; // COMPLIANT
+    i5 = g1;        // NON_COMPLIANT - block rather than statement scope matters
+    i4 = g1;        // NON_COMPLIANT
+    i6 = g2;        // COMPLIANT
+  }
+}
+
+void test_global_local_1() {
+  g1_1 = g2_1; // COMPLIANT
+}
+
+void test_structs() {
+  struct s1 *restrict p1 = &v1;
+  int *restrict px = &v1.x; // NON_COMPLIANT
+  {
+    int *restrict py;
+    int *restrict pz;
+    py = &v1.y; // COMPLIANT
+    py = (int *)0;
+    pz = &v1.z; // NON_COMPLIANT - block rather than statement scope matters
+    py = &v1.y; // NON_COMPLIANT
+  }
+}
+
+void copy(int *restrict p1, int *restrict p2, size_t s) {
+  for (size_t i = 0; i < s; ++i) {
+    p2[i] = p1[i];
+  }
+}
+
+void test_restrict_params() {
+  int i1 = 1;
+  int i2 = 2;
+  copy(&i1, &i1, 1); // NON_COMPLIANT
+  copy(&i1, &i2, 1); // COMPLIANT
+
+  int x[10];
+  int *px = &x[0];
+  copy(&x[0], &x[1], 1);       // COMPLIANT - non overlapping
+  copy(&x[0], &x[1], 2);       // NON_COMPLIANT - overlapping
+  copy(&x[0], (int *)x[0], 1); // COMPLIANT - non overlapping
+  copy(&x[0], px, 1);          // NON_COMPLIANT - overlapping
+}
+
+void test_strcpy() {
+  char s1[] = "my test string";
+  char s2[] = "my other string";
+  strcpy(&s1, &s1 + 3); // NON_COMPLIANT
+  strcpy(&s2, &s1);     // COMPLIANT
+}
+
+void test_memcpy() {
+  char s1[] = "my test string";
+  char s2[] = "my other string";
+  memcpy(&s1, &s1 + 3, 5); // NON_COMPLIANT
+  memcpy(&s2, &s1 + 3, 5); // COMPLIANT
+}
+
+void test_memmove() {
+  char s1[] = "my test string";
+  char s2[] = "my other string";
+  memmove(&s1, &s1 + 3, 5); // COMPLIANT - memmove is allowed to overlap
+  memmove(&s2, &s1 + 3, 5); // COMPLIANT
+}
+
+void test_scanf() {
+  char s1[200] = "%10s";
+  scanf(&s1, &s1 + 4); // NON_COMPLIANT
+}
+
+// TODO also consider the following:
+// strncpy(), strncpy_s()
+// strcat(), 	strcat_s()
+// strncat(),	strncat_s()
+// strtok_s()