Merge pull request github#4634 from geoffw0/modelchanges2

C++: Make classes in models.implementations private

Author: rdmarsh2

cpp/change-notes/2020-11-05-private-models.md

@@ -0,0 +1,3 @@
+lgtm,codescanning
+* Various classes in `semmle.code.cpp.models.implementations` have been made private. Users should not depend on library implementation details.
+* The `OperatorNewAllocationFunction`, `OperatorDeleteDeallocationFunction`, `Iterator` and `Snprintf` classes now have interfaces in `semmle.code.cpp.models.interfaces`.

cpp/ql/src/Critical/NewDelete.qll

@@ -5,8 +5,6 @@
 import cpp
 import semmle.code.cpp.controlflow.SSA
 import semmle.code.cpp.dataflow.DataFlow
-import semmle.code.cpp.models.implementations.Allocation
-import semmle.code.cpp.models.implementations.Deallocation
 
 /**
  * Holds if `alloc` is a use of `malloc` or `new`.  `kind` is

cpp/ql/src/Security/CWE/CWE-020/SafeExternalAPIFunction.qll

@@ -13,6 +13,7 @@ abstract class SafeExternalAPIFunction extends Function { }
 /** The default set of "safe" external APIs. */
 private class DefaultSafeExternalAPIFunction extends SafeExternalAPIFunction {
   DefaultSafeExternalAPIFunction() {
+    // implementation note: this should be based on the properties of public interfaces, rather than accessing implementation classes directly.  When we've done that, the three classes referenced here should be made fully private.
     this instanceof PureStrFunction or
     this instanceof StrLenFunction or
     this instanceof PureMemFunction

cpp/ql/src/Security/CWE/CWE-020/ir/SafeExternalAPIFunction.qll

@@ -13,6 +13,7 @@ abstract class SafeExternalAPIFunction extends Function { }
 /** The default set of "safe" external APIs. */
 private class DefaultSafeExternalAPIFunction extends SafeExternalAPIFunction {
   DefaultSafeExternalAPIFunction() {
+    // implementation note: this should be based on the properties of public interfaces, rather than accessing implementation classes directly.  When we've done that, the three classes referenced here should be made fully private.
     this instanceof PureStrFunction or
     this instanceof StrLenFunction or
     this instanceof PureMemFunction

cpp/ql/src/semmle/code/cpp/commons/Printf.qll

@@ -6,7 +6,6 @@ import semmle.code.cpp.Type
 import semmle.code.cpp.commons.CommonType
 import semmle.code.cpp.commons.StringAnalysis
 import semmle.code.cpp.models.interfaces.FormattingFunction
-import semmle.code.cpp.models.implementations.Printf
 
 class PrintfFormatAttribute extends FormatAttribute {
   PrintfFormatAttribute() { getArchetype() = ["printf", "__printf__"] }

cpp/ql/src/semmle/code/cpp/exprs/Expr.qll

@@ -6,7 +6,6 @@ import semmle.code.cpp.Element
 private import semmle.code.cpp.Enclosing
 private import semmle.code.cpp.internal.ResolveClass
 private import semmle.code.cpp.internal.AddressConstantExpression
-private import semmle.code.cpp.models.implementations.Allocation
 
 /**
  * A C/C++ expression.

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

@@ -10,7 +10,7 @@ import semmle.code.cpp.models.interfaces.Allocation
  * An allocation function (such as `malloc`) that has an argument for the size
  * in bytes.
  */
-class MallocAllocationFunction extends AllocationFunction {
+private class MallocAllocationFunction extends AllocationFunction {
   int sizeArg;
 
   MallocAllocationFunction() {
@@ -112,7 +112,7 @@ class MallocAllocationFunction extends AllocationFunction {
  * An allocation function (such as `alloca`) that does not require a
  * corresponding free (and has an argument for the size in bytes).
  */
-class AllocaAllocationFunction extends AllocationFunction {
+private class AllocaAllocationFunction extends AllocationFunction {
   int sizeArg;
 
   AllocaAllocationFunction() {
@@ -137,7 +137,7 @@ class AllocaAllocationFunction extends AllocationFunction {
  * An allocation function (such as `calloc`) that has an argument for the size
  * and another argument for the size of those units (in bytes).
  */
-class CallocAllocationFunction extends AllocationFunction {
+private class CallocAllocationFunction extends AllocationFunction {
   int sizeArg;
   int multArg;
 
@@ -158,7 +158,7 @@ class CallocAllocationFunction extends AllocationFunction {
  * An allocation function (such as `realloc`) that has an argument for the size
  * in bytes, and an argument for an existing pointer that is to be reallocated.
  */
-class ReallocAllocationFunction extends AllocationFunction {
+private class ReallocAllocationFunction extends AllocationFunction {
   int sizeArg;
   int reallocArg;
 
@@ -197,7 +197,7 @@ class ReallocAllocationFunction extends AllocationFunction {
  * A miscellaneous allocation function that has no explicit argument for
  * the size of the allocation.
  */
-class SizelessAllocationFunction extends AllocationFunction {
+private class SizelessAllocationFunction extends AllocationFunction {
   SizelessAllocationFunction() {
     exists(string name |
       hasGlobalName(name) and
@@ -236,40 +236,6 @@ class SizelessAllocationFunction extends AllocationFunction {
   }
 }
 
-/**
- * An `operator new` or `operator new[]` function that may be associated with `new` or
- * `new[]` expressions.  Note that `new` and `new[]` are not function calls, but these
- * functions may also be called directly.
- */
-class OperatorNewAllocationFunction extends AllocationFunction {
-  OperatorNewAllocationFunction() {
-    exists(string name |
-      hasGlobalName(name) and
-      (
-        // operator new(bytes, ...)
-        name = "operator new"
-        or
-        // operator new[](bytes, ...)
-        name = "operator new[]"
-      )
-    )
-  }
-
-  override int getSizeArg() { result = 0 }
-
-  override predicate requiresDealloc() { not exists(getPlacementArgument()) }
-
-  /**
-   * Gets the position of the placement pointer if this is a placement
-   * `operator new` function.
-   */
-  int getPlacementArgument() {
-    getNumberOfParameters() = 2 and
-    getParameter(1).getType() instanceof VoidPointerType and
-    result = 1
-  }
-}
-
 /**
  * Holds if `sizeExpr` is an expression consisting of a subexpression
  * `lengthExpr` multiplied by a constant `sizeof` that is the result of a
@@ -302,7 +268,7 @@ private predicate deconstructSizeExpr(Expr sizeExpr, Expr lengthExpr, int sizeof
 /**
  * An allocation expression that is a function call, such as call to `malloc`.
  */
-class CallAllocationExpr extends AllocationExpr, FunctionCall {
+private class CallAllocationExpr extends AllocationExpr, FunctionCall {
   AllocationFunction target;
 
   CallAllocationExpr() {
@@ -353,7 +319,7 @@ class CallAllocationExpr extends AllocationExpr, FunctionCall {
 /**
  * An allocation expression that is a `new` expression.
  */
-class NewAllocationExpr extends AllocationExpr, NewExpr {
+private class NewAllocationExpr extends AllocationExpr, NewExpr {
   NewAllocationExpr() { this instanceof NewExpr }
 
   override int getSizeBytes() { result = getAllocatedType().getSize() }
@@ -366,7 +332,7 @@ class NewAllocationExpr extends AllocationExpr, NewExpr {
 /**
  * An allocation expression that is a `new []` expression.
  */
-class NewArrayAllocationExpr extends AllocationExpr, NewArrayExpr {
+private class NewArrayAllocationExpr extends AllocationExpr, NewArrayExpr {
   NewArrayAllocationExpr() { this instanceof NewArrayExpr }
 
   override Expr getSizeExpr() {

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

@@ -9,7 +9,7 @@ import semmle.code.cpp.models.interfaces.Deallocation
 /**
  * A deallocation function such as `free`.
  */
-class StandardDeallocationFunction extends DeallocationFunction {
+private class StandardDeallocationFunction extends DeallocationFunction {
   int freedArg;
 
   StandardDeallocationFunction() {
@@ -89,32 +89,10 @@ class StandardDeallocationFunction extends DeallocationFunction {
   override int getFreedArg() { result = freedArg }
 }
 
-/**
- * An `operator delete` or `operator delete[]` function that may be associated
- * with `delete` or `delete[]` expressions.  Note that `delete` and `delete[]`
- * are not function calls, but these functions may also be called directly.
- */
-class OperatorDeleteDeallocationFunction extends DeallocationFunction {
-  OperatorDeleteDeallocationFunction() {
-    exists(string name |
-      hasGlobalName(name) and
-      (
-        // operator delete(pointer, ...)
-        name = "operator delete"
-        or
-        // operator delete[](pointer, ...)
-        name = "operator delete[]"
-      )
-    )
-  }
-
-  override int getFreedArg() { result = 0 }
-}
-
 /**
  * An deallocation expression that is a function call, such as call to `free`.
  */
-class CallDeallocationExpr extends DeallocationExpr, FunctionCall {
+private class CallDeallocationExpr extends DeallocationExpr, FunctionCall {
   DeallocationFunction target;
 
   CallDeallocationExpr() { target = getTarget() }
@@ -125,7 +103,7 @@ class CallDeallocationExpr extends DeallocationExpr, FunctionCall {
 /**
  * An deallocation expression that is a `delete` expression.
  */
-class DeleteDeallocationExpr extends DeallocationExpr, DeleteExpr {
+private class DeleteDeallocationExpr extends DeallocationExpr, DeleteExpr {
   DeleteDeallocationExpr() { this instanceof DeleteExpr }
 
   override Expr getFreedExpr() { result = getExpr() }
@@ -134,7 +112,7 @@ class DeleteDeallocationExpr extends DeallocationExpr, DeleteExpr {
 /**
  * An deallocation expression that is a `delete []` expression.
  */
-class DeleteArrayDeallocationExpr extends DeallocationExpr, DeleteArrayExpr {
+private class DeleteArrayDeallocationExpr extends DeallocationExpr, DeleteArrayExpr {
   DeleteArrayDeallocationExpr() { this instanceof DeleteArrayExpr }
 
   override Expr getFreedExpr() { result = getExpr() }

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

@@ -1,7 +1,7 @@
 import semmle.code.cpp.models.interfaces.Alias
 import semmle.code.cpp.models.interfaces.FlowSource
 
-class Fread extends AliasFunction, RemoteFlowFunction {
+private class Fread extends AliasFunction, RemoteFlowFunction {
   Fread() { this.hasGlobalName("fread") }
 
   override predicate parameterNeverEscapes(int n) {

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

@@ -6,7 +6,8 @@ import semmle.code.cpp.models.interfaces.FlowSource
 /**
  * The standard functions `getdelim`, `getwdelim` and the glibc variant `__getdelim`.
  */
-class GetDelimFunction extends TaintFunction, AliasFunction, SideEffectFunction, RemoteFlowFunction {
+private class GetDelimFunction extends TaintFunction, AliasFunction, SideEffectFunction,
+  RemoteFlowFunction {
   GetDelimFunction() { hasGlobalName(["getdelim", "getwdelim", "__getdelim"]) }
 
   override predicate hasTaintFlow(FunctionInput i, FunctionOutput o) {