From 7f54379a0cc0e1e81ce771f594955a3198ff935e Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Wed, 4 Nov 2020 08:56:58 +0000
Subject: [PATCH 01/15] C++: Make more function models private (except a few
 that are used outside the library).

---
 .../cpp/models/implementations/Allocation.qll | 16 +++---
 .../models/implementations/Deallocation.qll   |  8 +--
 .../code/cpp/models/implementations/Fread.qll |  2 +-
 .../cpp/models/implementations/GetDelim.qll   |  2 +-
 .../code/cpp/models/implementations/Gets.qll  |  2 +-
 .../implementations/IdentityFunction.qll      |  2 +-
 .../code/cpp/models/implementations/Inet.qll  | 20 +++----
 .../cpp/models/implementations/Iterator.qll   | 32 +++++------
 .../models/implementations/MemberFunction.qll | 10 ++--
 .../cpp/models/implementations/Memcpy.qll     |  2 +-
 .../cpp/models/implementations/Memset.qll     |  2 +-
 .../cpp/models/implementations/Printf.qll     |  6 +-
 .../code/cpp/models/implementations/Pure.qll  |  6 +-
 .../models/implementations/SmartPointer.qll   |  8 +--
 .../models/implementations/StdContainer.qll   | 16 +++---
 .../cpp/models/implementations/StdMap.qll     | 20 +++----
 .../cpp/models/implementations/StdSet.qll     | 16 +++---
 .../cpp/models/implementations/StdString.qll  | 56 +++++++++----------
 .../cpp/models/implementations/Strdup.qll     |  4 +-
 .../cpp/models/implementations/Strftime.qll   |  2 +-
 .../code/cpp/models/implementations/Swap.qll  |  2 +-
 21 files changed, 117 insertions(+), 117 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll
index 1f82b90bff42..b1275733a975 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll
+++ b/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
@@ -302,7 +302,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 +353,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 +366,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() {
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll
index 4f0341b673ee..c87ac7485df3 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll
+++ b/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() {
@@ -114,7 +114,7 @@ class OperatorDeleteDeallocationFunction extends DeallocationFunction {
 /**
  * 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 +125,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 +134,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() }
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Fread.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Fread.qll
index 4813e5c0066b..11568c8a9748 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Fread.qll
+++ b/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) {
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/GetDelim.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/GetDelim.qll
index 3f376cf2ff01..57fac090f57c 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/GetDelim.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/GetDelim.qll
@@ -6,7 +6,7 @@ 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) {
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Gets.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Gets.qll
index d22f2f376806..91653cd6b37f 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Gets.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Gets.qll
@@ -13,7 +13,7 @@ import semmle.code.cpp.models.interfaces.FlowSource
 /**
  * The standard functions `gets` and `fgets`.
  */
-class GetsFunction extends DataFlowFunction, TaintFunction, ArrayFunction, AliasFunction,
+private class GetsFunction extends DataFlowFunction, TaintFunction, ArrayFunction, AliasFunction,
   SideEffectFunction, RemoteFlowFunction {
   GetsFunction() {
     // gets(str)
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/IdentityFunction.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/IdentityFunction.qll
index f9eab7aba939..0178cd0f7ec3 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/IdentityFunction.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/IdentityFunction.qll
@@ -6,7 +6,7 @@ import semmle.code.cpp.models.interfaces.SideEffect
 /**
  * The standard function templates `std::move` and `std::forward`.
  */
-class IdentityFunction extends DataFlowFunction, SideEffectFunction, AliasFunction {
+private class IdentityFunction extends DataFlowFunction, SideEffectFunction, AliasFunction {
   IdentityFunction() {
     this.getNamespace().getParentNamespace() instanceof GlobalNamespace and
     this.getNamespace().getName() = "std" and
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Inet.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Inet.qll
index 58d8b66c91a5..736945468ede 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Inet.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Inet.qll
@@ -1,7 +1,7 @@
 import semmle.code.cpp.models.interfaces.Taint
 import semmle.code.cpp.models.interfaces.ArrayFunction
 
-class InetNtoa extends TaintFunction {
+private class InetNtoa extends TaintFunction {
   InetNtoa() { hasGlobalName("inet_ntoa") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -10,7 +10,7 @@ class InetNtoa extends TaintFunction {
   }
 }
 
-class InetAton extends TaintFunction, ArrayFunction {
+private class InetAton extends TaintFunction, ArrayFunction {
   InetAton() { hasGlobalName("inet_aton") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -30,7 +30,7 @@ class InetAton extends TaintFunction, ArrayFunction {
   }
 }
 
-class InetAddr extends TaintFunction, ArrayFunction, AliasFunction {
+private class InetAddr extends TaintFunction, ArrayFunction, AliasFunction {
   InetAddr() { hasGlobalName("inet_addr") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -49,7 +49,7 @@ class InetAddr extends TaintFunction, ArrayFunction, AliasFunction {
   override predicate parameterIsAlwaysReturned(int index) { none() }
 }
 
-class InetNetwork extends TaintFunction, ArrayFunction {
+private class InetNetwork extends TaintFunction, ArrayFunction {
   InetNetwork() { hasGlobalName("inet_network") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -62,7 +62,7 @@ class InetNetwork extends TaintFunction, ArrayFunction {
   override predicate hasArrayWithNullTerminator(int bufParam) { bufParam = 0 }
 }
 
-class InetMakeaddr extends TaintFunction {
+private class InetMakeaddr extends TaintFunction {
   InetMakeaddr() { hasGlobalName("inet_makeaddr") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -74,7 +74,7 @@ class InetMakeaddr extends TaintFunction {
   }
 }
 
-class InetLnaof extends TaintFunction {
+private class InetLnaof extends TaintFunction {
   InetLnaof() { hasGlobalName("inet_lnaof") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -83,7 +83,7 @@ class InetLnaof extends TaintFunction {
   }
 }
 
-class InetNetof extends TaintFunction {
+private class InetNetof extends TaintFunction {
   InetNetof() { hasGlobalName("inet_netof") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -92,7 +92,7 @@ class InetNetof extends TaintFunction {
   }
 }
 
-class InetPton extends TaintFunction, ArrayFunction {
+private class InetPton extends TaintFunction, ArrayFunction {
   InetPton() { hasGlobalName("inet_pton") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -112,7 +112,7 @@ class InetPton extends TaintFunction, ArrayFunction {
   override predicate hasArrayWithUnknownSize(int bufParam) { bufParam = 2 }
 }
 
-class Gethostbyname extends TaintFunction, ArrayFunction {
+private class Gethostbyname extends TaintFunction, ArrayFunction {
   Gethostbyname() { hasGlobalName("gethostbyname") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -125,7 +125,7 @@ class Gethostbyname extends TaintFunction, ArrayFunction {
   override predicate hasArrayWithNullTerminator(int bufParam) { bufParam = 0 }
 }
 
-class Gethostbyaddr extends TaintFunction, ArrayFunction {
+private class Gethostbyaddr extends TaintFunction, ArrayFunction {
   Gethostbyaddr() { hasGlobalName("gethostbyaddr") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll
index 804587f15654..7fb40b54a6e5 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll
@@ -13,7 +13,7 @@ import semmle.code.cpp.models.interfaces.Iterator
 /**
  * An instantiation of the `std::iterator_traits` template.
  */
-class IteratorTraits extends Class {
+private class IteratorTraits extends Class {
   IteratorTraits() {
     this.hasQualifiedName("std", "iterator_traits") and
     not this instanceof TemplateClass and
@@ -29,7 +29,7 @@ class IteratorTraits extends Class {
 /**
  * A type which has the typedefs expected for an iterator.
  */
-class IteratorByTypedefs extends Class {
+private class IteratorByTypedefs extends Class {
   IteratorByTypedefs() {
     this.getAMember().(TypedefType).hasName("difference_type") and
     this.getAMember().(TypedefType).hasName("value_type") and
@@ -43,7 +43,7 @@ class IteratorByTypedefs extends Class {
 /**
  * The `std::iterator` class.
  */
-class StdIterator extends Class {
+private class StdIterator extends Class {
   StdIterator() { this.hasQualifiedName("std", "iterator") }
 }
 
@@ -81,7 +81,7 @@ private FunctionInput getIteratorArgumentInput(Operator op, int index) {
 /**
  * A non-member prefix `operator*` function for an iterator type.
  */
-class IteratorPointerDereferenceOperator extends Operator, TaintFunction, IteratorReferenceFunction {
+private class IteratorPointerDereferenceOperator extends Operator, TaintFunction, IteratorReferenceFunction {
   FunctionInput iteratorInput;
 
   IteratorPointerDereferenceOperator() {
@@ -101,7 +101,7 @@ class IteratorPointerDereferenceOperator extends Operator, TaintFunction, Iterat
 /**
  * A non-member `operator++` or `operator--` function for an iterator type.
  */
-class IteratorCrementOperator extends Operator, DataFlowFunction {
+private class IteratorCrementOperator extends Operator, DataFlowFunction {
   FunctionInput iteratorInput;
 
   IteratorCrementOperator() {
@@ -118,7 +118,7 @@ class IteratorCrementOperator extends Operator, DataFlowFunction {
 /**
  * A non-member `operator+` function for an iterator type.
  */
-class IteratorAddOperator extends Operator, TaintFunction {
+private class IteratorAddOperator extends Operator, TaintFunction {
   FunctionInput iteratorInput;
 
   IteratorAddOperator() {
@@ -135,7 +135,7 @@ class IteratorAddOperator extends Operator, TaintFunction {
 /**
  * A non-member `operator-` function that takes a pointer difference type as its second argument.
  */
-class IteratorSubOperator extends Operator, TaintFunction {
+private class IteratorSubOperator extends Operator, TaintFunction {
   FunctionInput iteratorInput;
 
   IteratorSubOperator() {
@@ -153,7 +153,7 @@ class IteratorSubOperator extends Operator, TaintFunction {
 /**
  * A non-member `operator+=` or `operator-=` function for an iterator type.
  */
-class IteratorAssignArithmeticOperator extends Operator, DataFlowFunction, TaintFunction {
+private class IteratorAssignArithmeticOperator extends Operator, DataFlowFunction, TaintFunction {
   IteratorAssignArithmeticOperator() {
     this.hasName(["operator+=", "operator-="]) and
     this.getDeclaringType() instanceof Iterator
@@ -192,7 +192,7 @@ class IteratorPointerDereferenceMemberOperator extends MemberFunction, TaintFunc
 /**
  * An `operator++` or `operator--` member function for an iterator type.
  */
-class IteratorCrementMemberOperator extends MemberFunction, DataFlowFunction, TaintFunction {
+private class IteratorCrementMemberOperator extends MemberFunction, DataFlowFunction, TaintFunction {
   IteratorCrementMemberOperator() {
     this.hasName(["operator++", "operator--"]) and
     this.getDeclaringType() instanceof Iterator
@@ -215,7 +215,7 @@ class IteratorCrementMemberOperator extends MemberFunction, DataFlowFunction, Ta
 /**
  * A member `operator->` function for an iterator type.
  */
-class IteratorFieldMemberOperator extends Operator, TaintFunction {
+private class IteratorFieldMemberOperator extends Operator, TaintFunction {
   IteratorFieldMemberOperator() {
     this.hasName("operator->") and
     this.getDeclaringType() instanceof Iterator
@@ -230,7 +230,7 @@ class IteratorFieldMemberOperator extends Operator, TaintFunction {
 /**
  * An `operator+` or `operator-` member function of an iterator class.
  */
-class IteratorBinaryArithmeticMemberOperator extends MemberFunction, TaintFunction {
+private class IteratorBinaryArithmeticMemberOperator extends MemberFunction, TaintFunction {
   IteratorBinaryArithmeticMemberOperator() {
     this.hasName(["operator+", "operator-"]) and
     this.getDeclaringType() instanceof Iterator
@@ -245,7 +245,7 @@ class IteratorBinaryArithmeticMemberOperator extends MemberFunction, TaintFuncti
 /**
  * An `operator+=` or `operator-=` member function of an iterator class.
  */
-class IteratorAssignArithmeticMemberOperator extends MemberFunction, DataFlowFunction, TaintFunction {
+private class IteratorAssignArithmeticMemberOperator extends MemberFunction, DataFlowFunction, TaintFunction {
   IteratorAssignArithmeticMemberOperator() {
     this.hasName(["operator+=", "operator-="]) and
     this.getDeclaringType() instanceof Iterator
@@ -268,7 +268,7 @@ class IteratorAssignArithmeticMemberOperator extends MemberFunction, DataFlowFun
 /**
  * An `operator[]` member function of an iterator class.
  */
-class IteratorArrayMemberOperator extends MemberFunction, TaintFunction, IteratorReferenceFunction {
+private class IteratorArrayMemberOperator extends MemberFunction, TaintFunction, IteratorReferenceFunction {
   IteratorArrayMemberOperator() {
     this.hasName("operator[]") and
     this.getDeclaringType() instanceof Iterator
@@ -287,7 +287,7 @@ class IteratorArrayMemberOperator extends MemberFunction, TaintFunction, Iterato
  * The `hasTaintFlow` override provides flow through output iterators that return themselves with
  * `operator*` and use their own `operator=` to assign to the container.
  */
-class IteratorAssignmentMemberOperator extends MemberFunction, TaintFunction {
+private class IteratorAssignmentMemberOperator extends MemberFunction, TaintFunction {
   IteratorAssignmentMemberOperator() {
     this.hasName("operator=") and
     this.getDeclaringType() instanceof Iterator and
@@ -305,7 +305,7 @@ class IteratorAssignmentMemberOperator extends MemberFunction, TaintFunction {
  * A `begin` or `end` member function, or a related member function, that
  * returns an iterator.
  */
-class BeginOrEndFunction extends MemberFunction, TaintFunction, GetIteratorFunction {
+private class BeginOrEndFunction extends MemberFunction, TaintFunction, GetIteratorFunction {
   BeginOrEndFunction() {
     this
         .hasName(["begin", "cbegin", "rbegin", "crbegin", "end", "cend", "rend", "crend",
@@ -328,7 +328,7 @@ class BeginOrEndFunction extends MemberFunction, TaintFunction, GetIteratorFunct
  * The `std::front_inserter`, `std::inserter`, and `std::back_inserter`
  * functions.
  */
-class InserterIteratorFunction extends GetIteratorFunction {
+private class InserterIteratorFunction extends GetIteratorFunction {
   InserterIteratorFunction() {
     this.hasQualifiedName("std", ["front_inserter", "inserter", "back_inserter"])
   }
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/MemberFunction.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/MemberFunction.qll
index 0e4812cc25cc..c91795853587 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/MemberFunction.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/MemberFunction.qll
@@ -12,7 +12,7 @@ import semmle.code.cpp.models.interfaces.Taint
  * it does correspond with the constructors we are confident taint should flow
  * through.
  */
-class ConversionConstructorModel extends Constructor, TaintFunction {
+private class ConversionConstructorModel extends Constructor, TaintFunction {
   ConversionConstructorModel() {
     strictcount(Parameter p | p = getAParameter() and not p.hasInitializer()) = 1 and
     not hasSpecifier("explicit")
@@ -32,7 +32,7 @@ class ConversionConstructorModel extends Constructor, TaintFunction {
 /**
  * Model for C++ copy constructors.
  */
-class CopyConstructorModel extends CopyConstructor, DataFlowFunction {
+private class CopyConstructorModel extends CopyConstructor, DataFlowFunction {
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
     // data flow from the first constructor argument to the returned object
     input.isParameter(0) and
@@ -47,7 +47,7 @@ class CopyConstructorModel extends CopyConstructor, DataFlowFunction {
 /**
  * Model for C++ move constructors.
  */
-class MoveConstructorModel extends MoveConstructor, DataFlowFunction {
+private class MoveConstructorModel extends MoveConstructor, DataFlowFunction {
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
     // data flow from the first constructor argument to the returned object
     input.isParameter(0) and
@@ -62,7 +62,7 @@ class MoveConstructorModel extends MoveConstructor, DataFlowFunction {
 /**
  * Model for C++ copy assignment operators.
  */
-class CopyAssignmentOperatorModel extends CopyAssignmentOperator, TaintFunction {
+private class CopyAssignmentOperatorModel extends CopyAssignmentOperator, TaintFunction {
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     // taint flow from argument to self
     input.isParameterDeref(0) and
@@ -78,7 +78,7 @@ class CopyAssignmentOperatorModel extends CopyAssignmentOperator, TaintFunction
 /**
  * Model for C++ move assignment operators.
  */
-class MoveAssignmentOperatorModel extends MoveAssignmentOperator, TaintFunction {
+private class MoveAssignmentOperatorModel extends MoveAssignmentOperator, TaintFunction {
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     // taint flow from argument to self
     input.isParameterDeref(0) and
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Memcpy.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Memcpy.qll
index b6dac9e2d0bc..813c28345860 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Memcpy.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Memcpy.qll
@@ -13,7 +13,7 @@ import semmle.code.cpp.models.interfaces.Taint
  * The standard functions `memcpy`, `memmove` and `bcopy`; and the gcc variant
  * `__builtin___memcpy_chk`.
  */
-class MemcpyFunction extends ArrayFunction, DataFlowFunction, SideEffectFunction {
+private class MemcpyFunction extends ArrayFunction, DataFlowFunction, SideEffectFunction {
   MemcpyFunction() {
     // memcpy(dest, src, num)
     // memmove(dest, src, num)
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Memset.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Memset.qll
index 2c34369aee44..2fba6cfaf96c 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Memset.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Memset.qll
@@ -12,7 +12,7 @@ import semmle.code.cpp.models.interfaces.SideEffect
 /**
  * The standard function `memset` and its assorted variants
  */
-class MemsetFunction extends ArrayFunction, DataFlowFunction, AliasFunction, SideEffectFunction {
+private class MemsetFunction extends ArrayFunction, DataFlowFunction, AliasFunction, SideEffectFunction {
   MemsetFunction() {
     hasGlobalName("memset") or
     hasGlobalName("wmemset") or
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll
index 61dd3bc50b9d..50b9bac576c1 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll
@@ -62,7 +62,7 @@ class Fprintf extends FormattingFunction {
 /**
  * The standard function `sprintf` and its Microsoft and glib variants.
  */
-class Sprintf extends FormattingFunction {
+private class Sprintf extends FormattingFunction {
   Sprintf() {
     this instanceof TopLevelFunction and
     (
@@ -122,7 +122,7 @@ class Sprintf extends FormattingFunction {
  * The standard functions `snprintf` and `swprintf`, and their
  * Microsoft and glib variants.
  */
-class Snprintf extends FormattingFunction {
+private class Snprintf extends FormattingFunction {
   Snprintf() {
     this instanceof TopLevelFunction and
     (
@@ -201,7 +201,7 @@ class Snprintf extends FormattingFunction {
 /**
  * The Microsoft `StringCchPrintf` function and variants.
  */
-class StringCchPrintf extends FormattingFunction {
+private class StringCchPrintf extends FormattingFunction {
   StringCchPrintf() {
     this instanceof TopLevelFunction and
     (
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll
index c85983551b7b..3a779f11f55a 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll
@@ -3,7 +3,7 @@ import semmle.code.cpp.models.interfaces.Taint
 import semmle.code.cpp.models.interfaces.Alias
 import semmle.code.cpp.models.interfaces.SideEffect
 
-class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunction, SideEffectFunction {
+private class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunction, SideEffectFunction {
   PureStrFunction() {
     hasGlobalOrStdName(["atof", "atoi", "atol", "atoll", "strcasestr", "strchnul", "strchr",
           "strchrnul", "strstr", "strpbrk", "strcmp", "strcspn", "strncmp", "strrchr", "strspn",
@@ -56,7 +56,7 @@ class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunction, SideE
   }
 }
 
-class StrLenFunction extends AliasFunction, ArrayFunction, SideEffectFunction {
+private class StrLenFunction extends AliasFunction, ArrayFunction, SideEffectFunction {
   StrLenFunction() {
     hasGlobalOrStdName(["strlen", "strnlen", "wcslen"])
     or
@@ -89,7 +89,7 @@ class StrLenFunction extends AliasFunction, ArrayFunction, SideEffectFunction {
   }
 }
 
-class PureFunction extends TaintFunction, SideEffectFunction {
+private class PureFunction extends TaintFunction, SideEffectFunction {
   PureFunction() { hasGlobalOrStdName(["abs", "labs"]) }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/SmartPointer.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/SmartPointer.qll
index 30a0fdfb34bb..1bc8fa0a91d4 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/SmartPointer.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/SmartPointer.qll
@@ -3,14 +3,14 @@ import semmle.code.cpp.models.interfaces.Taint
 /**
  * The `std::shared_ptr` and `std::unique_ptr` template classes.
  */
-class UniqueOrSharedPtr extends Class {
+private class UniqueOrSharedPtr extends Class {
   UniqueOrSharedPtr() { this.hasQualifiedName("std", ["shared_ptr", "unique_ptr"]) }
 }
 
 /**
  * The `std::make_shared` and `std::make_unique` template functions.
  */
-class MakeUniqueOrShared extends TaintFunction {
+private class MakeUniqueOrShared extends TaintFunction {
   MakeUniqueOrShared() { this.hasQualifiedName("std", ["make_shared", "make_unique"]) }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -33,7 +33,7 @@ class MakeUniqueOrShared extends TaintFunction {
 /**
  * A prefix `operator*` member function for a `shared_ptr` or `unique_ptr` type.
  */
-class UniqueOrSharedDereferenceMemberOperator extends MemberFunction, TaintFunction {
+private class UniqueOrSharedDereferenceMemberOperator extends MemberFunction, TaintFunction {
   UniqueOrSharedDereferenceMemberOperator() {
     this.hasName("operator*") and
     this.getDeclaringType() instanceof UniqueOrSharedPtr
@@ -48,7 +48,7 @@ class UniqueOrSharedDereferenceMemberOperator extends MemberFunction, TaintFunct
 /**
  * The `std::shared_ptr` or `std::unique_ptr` function `get`.
  */
-class UniqueOrSharedGet extends TaintFunction {
+private class UniqueOrSharedGet extends TaintFunction {
   UniqueOrSharedGet() {
     this.hasName("get") and
     this.getDeclaringType() instanceof UniqueOrSharedPtr
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/StdContainer.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/StdContainer.qll
index a339dadb860f..b49a4be0bd35 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/StdContainer.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/StdContainer.qll
@@ -13,7 +13,7 @@ import semmle.code.cpp.models.implementations.Iterator
  * std::vector<std::string> v(100, potentially_tainted_string);
  * ```
  */
-class StdSequenceContainerConstructor extends Constructor, TaintFunction {
+private class StdSequenceContainerConstructor extends Constructor, TaintFunction {
   StdSequenceContainerConstructor() {
     this.getDeclaringType().hasQualifiedName("std", ["vector", "deque", "list", "forward_list"])
   }
@@ -49,7 +49,7 @@ class StdSequenceContainerConstructor extends Constructor, TaintFunction {
 /**
  * The standard container function `data`.
  */
-class StdSequenceContainerData extends TaintFunction {
+private class StdSequenceContainerData extends TaintFunction {
   StdSequenceContainerData() { this.hasQualifiedName("std", ["array", "vector"], "data") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -67,7 +67,7 @@ class StdSequenceContainerData extends TaintFunction {
 /**
  * The standard container functions `push_back` and `push_front`.
  */
-class StdSequenceContainerPush extends TaintFunction {
+private class StdSequenceContainerPush extends TaintFunction {
   StdSequenceContainerPush() {
     this.hasQualifiedName("std", "vector", "push_back") or
     this.hasQualifiedName("std", "deque", ["push_back", "push_front"]) or
@@ -85,7 +85,7 @@ class StdSequenceContainerPush extends TaintFunction {
 /**
  * The standard container functions `front` and `back`.
  */
-class StdSequenceContainerFrontBack extends TaintFunction {
+private class StdSequenceContainerFrontBack extends TaintFunction {
   StdSequenceContainerFrontBack() {
     this.hasQualifiedName("std", "array", ["front", "back"]) or
     this.hasQualifiedName("std", "vector", ["front", "back"]) or
@@ -104,7 +104,7 @@ class StdSequenceContainerFrontBack extends TaintFunction {
 /**
  * The standard container functions `insert` and `insert_after`.
  */
-class StdSequenceContainerInsert extends TaintFunction {
+private class StdSequenceContainerInsert extends TaintFunction {
   StdSequenceContainerInsert() {
     this.hasQualifiedName("std", ["vector", "deque", "list"], "insert") or
     this.hasQualifiedName("std", ["forward_list"], "insert_after")
@@ -141,7 +141,7 @@ class StdSequenceContainerInsert extends TaintFunction {
 /**
  * The standard container function `assign`.
  */
-class StdSequenceContainerAssign extends TaintFunction {
+private class StdSequenceContainerAssign extends TaintFunction {
   StdSequenceContainerAssign() {
     this.hasQualifiedName("std", ["vector", "deque", "list", "forward_list"], "assign")
   }
@@ -173,7 +173,7 @@ class StdSequenceContainerAssign extends TaintFunction {
 /**
  * The standard container `swap` functions.
  */
-class StdSequenceContainerSwap extends TaintFunction {
+private class StdSequenceContainerSwap extends TaintFunction {
   StdSequenceContainerSwap() {
     this.hasQualifiedName("std", ["array", "vector", "deque", "list", "forward_list"], "swap")
   }
@@ -191,7 +191,7 @@ class StdSequenceContainerSwap extends TaintFunction {
 /**
  * The standard container functions `at` and `operator[]`.
  */
-class StdSequenceContainerAt extends TaintFunction {
+private class StdSequenceContainerAt extends TaintFunction {
   StdSequenceContainerAt() {
     this.hasQualifiedName("std", ["vector", "array", "deque"], ["at", "operator[]"])
   }
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/StdMap.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/StdMap.qll
index 12e6624903e6..4c54a4988e5a 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/StdMap.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/StdMap.qll
@@ -8,7 +8,7 @@ import semmle.code.cpp.models.implementations.Iterator
 /**
  * Additional model for map constructors using iterator inputs.
  */
-class StdMapConstructor extends Constructor, TaintFunction {
+private class StdMapConstructor extends Constructor, TaintFunction {
   StdMapConstructor() {
     this.hasQualifiedName("std", "map", "map") or
     this.hasQualifiedName("std", "unordered_map", "unordered_map")
@@ -35,7 +35,7 @@ class StdMapConstructor extends Constructor, TaintFunction {
 /**
  * The standard map `insert` and `insert_or_assign` functions.
  */
-class StdMapInsert extends TaintFunction {
+private class StdMapInsert extends TaintFunction {
   StdMapInsert() {
     this.hasQualifiedName("std", ["map", "unordered_map"], ["insert", "insert_or_assign"])
   }
@@ -54,7 +54,7 @@ class StdMapInsert extends TaintFunction {
 /**
  * The standard map `emplace` and `emplace_hint` functions.
  */
-class StdMapEmplace extends TaintFunction {
+private class StdMapEmplace extends TaintFunction {
   StdMapEmplace() {
     this.hasQualifiedName("std", ["map", "unordered_map"], ["emplace", "emplace_hint"])
   }
@@ -78,7 +78,7 @@ class StdMapEmplace extends TaintFunction {
 /**
  * The standard map `try_emplace` function.
  */
-class StdMapTryEmplace extends TaintFunction {
+private class StdMapTryEmplace extends TaintFunction {
   StdMapTryEmplace() { this.hasQualifiedName("std", ["map", "unordered_map"], "try_emplace") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -105,7 +105,7 @@ class StdMapTryEmplace extends TaintFunction {
 /**
  * The standard map `swap` function.
  */
-class StdMapSwap extends TaintFunction {
+private class StdMapSwap extends TaintFunction {
   StdMapSwap() { this.hasQualifiedName("std", ["map", "unordered_map"], "swap") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -121,7 +121,7 @@ class StdMapSwap extends TaintFunction {
 /**
  * The standard map `merge` function.
  */
-class StdMapMerge extends TaintFunction {
+private class StdMapMerge extends TaintFunction {
   StdMapMerge() { this.hasQualifiedName("std", ["map", "unordered_map"], "merge") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -134,7 +134,7 @@ class StdMapMerge extends TaintFunction {
 /**
  * The standard map functions `at` and `operator[]`.
  */
-class StdMapAt extends TaintFunction {
+private class StdMapAt extends TaintFunction {
   StdMapAt() { this.hasQualifiedName("std", ["map", "unordered_map"], ["at", "operator[]"]) }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -151,7 +151,7 @@ class StdMapAt extends TaintFunction {
 /**
  * The standard map `find` function.
  */
-class StdMapFind extends TaintFunction {
+private class StdMapFind extends TaintFunction {
   StdMapFind() { this.hasQualifiedName("std", ["map", "unordered_map"], "find") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -163,7 +163,7 @@ class StdMapFind extends TaintFunction {
 /**
  * The standard map `erase` function.
  */
-class StdMapErase extends TaintFunction {
+private class StdMapErase extends TaintFunction {
   StdMapErase() { this.hasQualifiedName("std", ["map", "unordered_map"], "erase") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -177,7 +177,7 @@ class StdMapErase extends TaintFunction {
 /**
  * The standard map `lower_bound`, `upper_bound` and `equal_range` functions.
  */
-class StdMapEqualRange extends TaintFunction {
+private class StdMapEqualRange extends TaintFunction {
   StdMapEqualRange() {
     this
         .hasQualifiedName("std", ["map", "unordered_map"],
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/StdSet.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/StdSet.qll
index 06e8be4c4a41..d18258e86b0c 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/StdSet.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/StdSet.qll
@@ -8,7 +8,7 @@ import semmle.code.cpp.models.implementations.Iterator
 /**
  * Additional model for set constructors using iterator inputs.
  */
-class StdSetConstructor extends Constructor, TaintFunction {
+private class StdSetConstructor extends Constructor, TaintFunction {
   StdSetConstructor() {
     this.hasQualifiedName("std", "set", "set") or
     this.hasQualifiedName("std", "unordered_set", "unordered_set")
@@ -35,7 +35,7 @@ class StdSetConstructor extends Constructor, TaintFunction {
 /**
  * The standard set `insert` and `insert_or_assign` functions.
  */
-class StdSetInsert extends TaintFunction {
+private class StdSetInsert extends TaintFunction {
   StdSetInsert() { this.hasQualifiedName("std", ["set", "unordered_set"], "insert") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -52,7 +52,7 @@ class StdSetInsert extends TaintFunction {
 /**
  * The standard set `emplace` and `emplace_hint` functions.
  */
-class StdSetEmplace extends TaintFunction {
+private class StdSetEmplace extends TaintFunction {
   StdSetEmplace() {
     this.hasQualifiedName("std", ["set", "unordered_set"], ["emplace", "emplace_hint"])
   }
@@ -75,7 +75,7 @@ class StdSetEmplace extends TaintFunction {
 /**
  * The standard set `swap` functions.
  */
-class StdSetSwap extends TaintFunction {
+private class StdSetSwap extends TaintFunction {
   StdSetSwap() { this.hasQualifiedName("std", ["set", "unordered_set"], "swap") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -91,7 +91,7 @@ class StdSetSwap extends TaintFunction {
 /**
  * The standard set `merge` function.
  */
-class StdSetMerge extends TaintFunction {
+private class StdSetMerge extends TaintFunction {
   StdSetMerge() { this.hasQualifiedName("std", ["set", "unordered_set"], "merge") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -104,7 +104,7 @@ class StdSetMerge extends TaintFunction {
 /**
  * The standard set `find` function.
  */
-class StdSetFind extends TaintFunction {
+private class StdSetFind extends TaintFunction {
   StdSetFind() { this.hasQualifiedName("std", ["set", "unordered_set"], "find") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -116,7 +116,7 @@ class StdSetFind extends TaintFunction {
 /**
  * The standard set `erase` function.
  */
-class StdSetErase extends TaintFunction {
+private class StdSetErase extends TaintFunction {
   StdSetErase() { this.hasQualifiedName("std", ["set", "unordered_set"], "erase") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -130,7 +130,7 @@ class StdSetErase extends TaintFunction {
 /**
  * The standard set `lower_bound`, `upper_bound` and `equal_range` functions.
  */
-class StdSetEqualRange extends TaintFunction {
+private class StdSetEqualRange extends TaintFunction {
   StdSetEqualRange() {
     this
         .hasQualifiedName("std", ["set", "unordered_set"],
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll
index b8a000b963ea..62a19d8cb924 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll
@@ -10,7 +10,7 @@ import semmle.code.cpp.models.implementations.Iterator
 /**
  * The `std::basic_string` template class.
  */
-class StdBasicString extends TemplateClass {
+private class StdBasicString extends TemplateClass {
   StdBasicString() { this.hasQualifiedName("std", "basic_string") }
 }
 
@@ -22,7 +22,7 @@ class StdBasicString extends TemplateClass {
  * std::string b(a.begin(), a.end());
  * ```
  */
-class StdStringConstructor extends Constructor, TaintFunction {
+private class StdStringConstructor extends Constructor, TaintFunction {
   StdStringConstructor() { this.getDeclaringType().hasQualifiedName("std", "basic_string") }
 
   /**
@@ -58,7 +58,7 @@ class StdStringConstructor extends Constructor, TaintFunction {
 /**
  * The `std::string` function `c_str`.
  */
-class StdStringCStr extends TaintFunction {
+private class StdStringCStr extends TaintFunction {
   StdStringCStr() { this.hasQualifiedName("std", "basic_string", "c_str") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -71,7 +71,7 @@ class StdStringCStr extends TaintFunction {
 /**
  * The `std::string` function `data`.
  */
-class StdStringData extends TaintFunction {
+private class StdStringData extends TaintFunction {
   StdStringData() { this.hasQualifiedName("std", "basic_string", "data") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -89,7 +89,7 @@ class StdStringData extends TaintFunction {
 /**
  * The `std::string` function `push_back`.
  */
-class StdStringPush extends TaintFunction {
+private class StdStringPush extends TaintFunction {
   StdStringPush() { this.hasQualifiedName("std", "basic_string", "push_back") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -102,7 +102,7 @@ class StdStringPush extends TaintFunction {
 /**
  * The `std::string` functions `front` and `back`.
  */
-class StdStringFrontBack extends TaintFunction {
+private class StdStringFrontBack extends TaintFunction {
   StdStringFrontBack() { this.hasQualifiedName("std", "basic_string", ["front", "back"]) }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -115,7 +115,7 @@ class StdStringFrontBack extends TaintFunction {
 /**
  * The `std::string` function `operator+`.
  */
-class StdStringPlus extends TaintFunction {
+private class StdStringPlus extends TaintFunction {
   StdStringPlus() {
     this.hasQualifiedName("std", "operator+") and
     this.getUnspecifiedType() = any(StdBasicString s).getAnInstantiation()
@@ -136,7 +136,7 @@ class StdStringPlus extends TaintFunction {
  * `replace`. All of these functions combine the existing string
  * with a new string (or character) from one of the arguments.
  */
-class StdStringAppend extends TaintFunction {
+private class StdStringAppend extends TaintFunction {
   StdStringAppend() {
     this.hasQualifiedName("std", "basic_string", ["operator+=", "append", "insert", "replace"])
   }
@@ -179,7 +179,7 @@ class StdStringAppend extends TaintFunction {
 /**
  * The standard function `std::string.assign`.
  */
-class StdStringAssign extends TaintFunction {
+private class StdStringAssign extends TaintFunction {
   StdStringAssign() { this.hasQualifiedName("std", "basic_string", "assign") }
 
   /**
@@ -219,7 +219,7 @@ class StdStringAssign extends TaintFunction {
 /**
  * The standard function `std::string.copy`.
  */
-class StdStringCopy extends TaintFunction {
+private class StdStringCopy extends TaintFunction {
   StdStringCopy() { this.hasQualifiedName("std", "basic_string", "copy") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -232,7 +232,7 @@ class StdStringCopy extends TaintFunction {
 /**
  * The standard function `std::string.substr`.
  */
-class StdStringSubstr extends TaintFunction {
+private class StdStringSubstr extends TaintFunction {
   StdStringSubstr() { this.hasQualifiedName("std", "basic_string", "substr") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -245,7 +245,7 @@ class StdStringSubstr extends TaintFunction {
 /**
  * The standard functions `std::string.swap` and `std::stringstream::swap`.
  */
-class StdStringSwap extends TaintFunction {
+private class StdStringSwap extends TaintFunction {
   StdStringSwap() {
     this.hasQualifiedName("std", "basic_string", "swap") or
     this.hasQualifiedName("std", "basic_stringstream", "swap")
@@ -264,7 +264,7 @@ class StdStringSwap extends TaintFunction {
 /**
  * The `std::string` functions `at` and `operator[]`.
  */
-class StdStringAt extends TaintFunction {
+private class StdStringAt extends TaintFunction {
   StdStringAt() { this.hasQualifiedName("std", "basic_string", ["at", "operator[]"]) }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -281,14 +281,14 @@ class StdStringAt extends TaintFunction {
 /**
  * The `std::basic_istream` template class.
  */
-class StdBasicIStream extends TemplateClass {
+private class StdBasicIStream extends TemplateClass {
   StdBasicIStream() { this.hasQualifiedName("std", "basic_istream") }
 }
 
 /**
  * The `std::istream` function `operator>>` (defined as a member function).
  */
-class StdIStreamIn extends DataFlowFunction, TaintFunction {
+private class StdIStreamIn extends DataFlowFunction, TaintFunction {
   StdIStreamIn() { this.hasQualifiedName("std", "basic_istream", "operator>>") }
 
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
@@ -311,7 +311,7 @@ class StdIStreamIn extends DataFlowFunction, TaintFunction {
 /**
  * The `std::istream` function `operator>>` (defined as a non-member function).
  */
-class StdIStreamInNonMember extends DataFlowFunction, TaintFunction {
+private class StdIStreamInNonMember extends DataFlowFunction, TaintFunction {
   StdIStreamInNonMember() {
     this.hasQualifiedName("std", "operator>>") and
     this.getUnspecifiedType().(ReferenceType).getBaseType() =
@@ -338,7 +338,7 @@ class StdIStreamInNonMember extends DataFlowFunction, TaintFunction {
 /**
  * The `std::istream` functions `get` (without parameters) and `peek`.
  */
-class StdIStreamGet extends TaintFunction {
+private class StdIStreamGet extends TaintFunction {
   StdIStreamGet() {
     this.hasQualifiedName("std", "basic_istream", ["get", "peek"]) and
     this.getNumberOfParameters() = 0
@@ -354,7 +354,7 @@ class StdIStreamGet extends TaintFunction {
 /**
  * The `std::istream` functions `get` (with parameters) and `read`.
  */
-class StdIStreamRead extends DataFlowFunction, TaintFunction {
+private class StdIStreamRead extends DataFlowFunction, TaintFunction {
   StdIStreamRead() {
     this.hasQualifiedName("std", "basic_istream", ["get", "read"]) and
     this.getNumberOfParameters() > 0
@@ -380,7 +380,7 @@ class StdIStreamRead extends DataFlowFunction, TaintFunction {
 /**
  * The `std::istream` function `readsome`.
  */
-class StdIStreamReadSome extends TaintFunction {
+private class StdIStreamReadSome extends TaintFunction {
   StdIStreamReadSome() { this.hasQualifiedName("std", "basic_istream", "readsome") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -393,7 +393,7 @@ class StdIStreamReadSome extends TaintFunction {
 /**
  * The `std::istream` function `putback`.
  */
-class StdIStreamPutBack extends DataFlowFunction, TaintFunction {
+private class StdIStreamPutBack extends DataFlowFunction, TaintFunction {
   StdIStreamPutBack() { this.hasQualifiedName("std", "basic_istream", "putback") }
 
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
@@ -426,7 +426,7 @@ class StdIStreamPutBack extends DataFlowFunction, TaintFunction {
 /**
  * The `std::istream` function `getline`.
  */
-class StdIStreamGetLine extends DataFlowFunction, TaintFunction {
+private class StdIStreamGetLine extends DataFlowFunction, TaintFunction {
   StdIStreamGetLine() { this.hasQualifiedName("std", "basic_istream", "getline") }
 
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
@@ -449,7 +449,7 @@ class StdIStreamGetLine extends DataFlowFunction, TaintFunction {
 /**
  * The (non-member) function `std::getline`.
  */
-class StdGetLine extends DataFlowFunction, TaintFunction {
+private class StdGetLine extends DataFlowFunction, TaintFunction {
   StdGetLine() { this.hasQualifiedName("std", "getline") }
 
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
@@ -472,7 +472,7 @@ class StdGetLine extends DataFlowFunction, TaintFunction {
 /**
  * The `std::basic_ostream` template class.
  */
-class StdBasicOStream extends TemplateClass {
+private class StdBasicOStream extends TemplateClass {
   StdBasicOStream() { this.hasQualifiedName("std", "basic_ostream") }
 }
 
@@ -480,7 +480,7 @@ class StdBasicOStream extends TemplateClass {
  * The `std::ostream` functions `operator<<` (defined as a member function),
  * `put` and `write`.
  */
-class StdOStreamOut extends DataFlowFunction, TaintFunction {
+private class StdOStreamOut extends DataFlowFunction, TaintFunction {
   StdOStreamOut() { this.hasQualifiedName("std", "basic_ostream", ["operator<<", "put", "write"]) }
 
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
@@ -513,7 +513,7 @@ class StdOStreamOut extends DataFlowFunction, TaintFunction {
 /**
  * The `std::ostream` function `operator<<` (defined as a non-member function).
  */
-class StdOStreamOutNonMember extends DataFlowFunction, TaintFunction {
+private class StdOStreamOutNonMember extends DataFlowFunction, TaintFunction {
   StdOStreamOutNonMember() {
     this.hasQualifiedName("std", "operator<<") and
     this.getUnspecifiedType().(ReferenceType).getBaseType() =
@@ -545,7 +545,7 @@ class StdOStreamOutNonMember extends DataFlowFunction, TaintFunction {
  * Additional model for `std::stringstream` constructors that take a string
  * input parameter.
  */
-class StdStringStreamConstructor extends Constructor, TaintFunction {
+private class StdStringStreamConstructor extends Constructor, TaintFunction {
   StdStringStreamConstructor() {
     this.getDeclaringType().hasQualifiedName("std", "basic_stringstream")
   }
@@ -571,7 +571,7 @@ class StdStringStreamConstructor extends Constructor, TaintFunction {
 /**
  * The `std::stringstream` function `str`.
  */
-class StdStringStreamStr extends TaintFunction {
+private class StdStringStreamStr extends TaintFunction {
   StdStringStreamStr() { this.hasQualifiedName("std", "basic_stringstream", "str") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -589,7 +589,7 @@ class StdStringStreamStr extends TaintFunction {
  * A `std::` stream function that does not require a model, except that it
  * returns a reference to `*this` and thus could be used in a chain.
  */
-class StdStreamFunction extends DataFlowFunction, TaintFunction {
+private class StdStreamFunction extends DataFlowFunction, TaintFunction {
   StdStreamFunction() {
     this.hasQualifiedName("std", "basic_istream", ["ignore", "unget", "seekg"]) or
     this.hasQualifiedName("std", "basic_ostream", ["seekp", "flush"]) or
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Strdup.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Strdup.qll
index 3497ab9a0650..7f71dfc06477 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Strdup.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Strdup.qll
@@ -11,7 +11,7 @@ import semmle.code.cpp.models.interfaces.Taint
 /**
  * A `strdup` style allocation function.
  */
-class StrdupFunction extends AllocationFunction, ArrayFunction, DataFlowFunction {
+private class StrdupFunction extends AllocationFunction, ArrayFunction, DataFlowFunction {
   StrdupFunction() {
     exists(string name |
       hasGlobalName(name) and
@@ -47,7 +47,7 @@ class StrdupFunction extends AllocationFunction, ArrayFunction, DataFlowFunction
 /**
  * A `strndup` style allocation function.
  */
-class StrndupFunction extends AllocationFunction, ArrayFunction, DataFlowFunction {
+private class StrndupFunction extends AllocationFunction, ArrayFunction, DataFlowFunction {
   StrndupFunction() {
     exists(string name |
       hasGlobalName(name) and
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Strftime.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Strftime.qll
index 3e58fd8c2584..0dad89e950f1 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Strftime.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Strftime.qll
@@ -1,7 +1,7 @@
 import semmle.code.cpp.models.interfaces.Taint
 import semmle.code.cpp.models.interfaces.ArrayFunction
 
-class Strftime extends TaintFunction, ArrayFunction {
+private class Strftime extends TaintFunction, ArrayFunction {
   Strftime() { hasGlobalName("strftime") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Swap.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Swap.qll
index a7474501ad74..9fb1f4700226 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Swap.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Swap.qll
@@ -4,7 +4,7 @@ import semmle.code.cpp.models.interfaces.Taint
 /**
  * The standard function `swap`.
  */
-class Swap extends DataFlowFunction {
+private class Swap extends DataFlowFunction {
   Swap() { this.hasQualifiedName("std", "swap") }
 
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {

From b5326b39376c743092ad423b875be37443d60a3c Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Wed, 4 Nov 2020 08:56:58 +0000
Subject: [PATCH 02/15] C++: Give OperatorNewAllocationFunction,
 OperatorDeleteAllocationFunction proper interfaces.

---
 cpp/ql/src/Critical/NewDelete.qll                  |  2 --
 cpp/ql/src/semmle/code/cpp/exprs/Expr.qll          |  1 -
 .../code/cpp/models/implementations/Allocation.qll | 14 ++++----------
 .../cpp/models/implementations/Deallocation.qll    |  8 +++-----
 .../code/cpp/models/interfaces/Allocation.qll      | 13 +++++++++++++
 .../code/cpp/models/interfaces/Deallocation.qll    |  8 ++++++++
 6 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/cpp/ql/src/Critical/NewDelete.qll b/cpp/ql/src/Critical/NewDelete.qll
index 30b9f9ad94ad..77da1f36d80c 100644
--- a/cpp/ql/src/Critical/NewDelete.qll
+++ b/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
diff --git a/cpp/ql/src/semmle/code/cpp/exprs/Expr.qll b/cpp/ql/src/semmle/code/cpp/exprs/Expr.qll
index a39b215b54a2..15ee38031b55 100644
--- a/cpp/ql/src/semmle/code/cpp/exprs/Expr.qll
+++ b/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.
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll
index b1275733a975..008d0b4c8aa9 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll
@@ -237,12 +237,10 @@ private 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.
+ * Implements `OperatorNewAllocationFunction`.
  */
-class OperatorNewAllocationFunction extends AllocationFunction {
-  OperatorNewAllocationFunction() {
+private class OperatorNewAllocationFunctionImpl extends OperatorNewAllocationFunction {
+  OperatorNewAllocationFunctionImpl() {
     exists(string name |
       hasGlobalName(name) and
       (
@@ -259,11 +257,7 @@ class OperatorNewAllocationFunction extends AllocationFunction {
 
   override predicate requiresDealloc() { not exists(getPlacementArgument()) }
 
-  /**
-   * Gets the position of the placement pointer if this is a placement
-   * `operator new` function.
-   */
-  int getPlacementArgument() {
+  override int getPlacementArgument() {
     getNumberOfParameters() = 2 and
     getParameter(1).getType() instanceof VoidPointerType and
     result = 1
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll
index c87ac7485df3..7604c630f8b1 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll
@@ -90,12 +90,10 @@ private class StandardDeallocationFunction extends DeallocationFunction {
 }
 
 /**
- * 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.
+ * Implements `OperatorDeleteDeallocationFunction`.
  */
-class OperatorDeleteDeallocationFunction extends DeallocationFunction {
-  OperatorDeleteDeallocationFunction() {
+private class OperatorDeleteDeallocationFunctionImpl extends OperatorDeleteDeallocationFunction {
+  OperatorDeleteDeallocationFunctionImpl() {
     exists(string name |
       hasGlobalName(name) and
       (
diff --git a/cpp/ql/src/semmle/code/cpp/models/interfaces/Allocation.qll b/cpp/ql/src/semmle/code/cpp/models/interfaces/Allocation.qll
index 1ebf40b1f014..60671dc155f2 100644
--- a/cpp/ql/src/semmle/code/cpp/models/interfaces/Allocation.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/interfaces/Allocation.qll
@@ -85,3 +85,16 @@ abstract class AllocationExpr extends Expr {
    */
   predicate requiresDealloc() { any() }
 }
+
+/**
+ * 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.
+ */
+abstract class OperatorNewAllocationFunction extends AllocationFunction {
+  /**
+   * Gets the position of the placement pointer if this is a placement
+   * `operator new` function.
+   */
+  int getPlacementArgument() { none()}
+}
diff --git a/cpp/ql/src/semmle/code/cpp/models/interfaces/Deallocation.qll b/cpp/ql/src/semmle/code/cpp/models/interfaces/Deallocation.qll
index 23eca5164188..36efd0f9a234 100644
--- a/cpp/ql/src/semmle/code/cpp/models/interfaces/Deallocation.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/interfaces/Deallocation.qll
@@ -30,3 +30,11 @@ abstract class DeallocationExpr extends Expr {
    */
   Expr getFreedExpr() { none() }
 }
+
+/**
+ * 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.
+ */
+abstract class OperatorDeleteDeallocationFunction extends DeallocationFunction {
+}

From c9f846e0d2cf7cd0a604d17c714ef284daf3922b Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Thu, 5 Nov 2020 10:34:59 +0000
Subject: [PATCH 03/15] C++: Give Iterator a proper interface.

---
 .../src/semmle/code/cpp/models/implementations/Iterator.qll | 6 +++---
 .../semmle/code/cpp/models/implementations/StdContainer.qll | 2 +-
 .../src/semmle/code/cpp/models/implementations/StdMap.qll   | 2 +-
 .../src/semmle/code/cpp/models/implementations/StdSet.qll   | 2 +-
 .../semmle/code/cpp/models/implementations/StdString.qll    | 3 ++-
 cpp/ql/src/semmle/code/cpp/models/interfaces/Iterator.qll   | 6 ++++++
 6 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll
index 7fb40b54a6e5..d8334017f864 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll
@@ -48,10 +48,10 @@ private class StdIterator extends Class {
 }
 
 /**
- * A type which can be used as an iterator
+ * Implements `Iterator`.
  */
-class Iterator extends Type {
-  Iterator() {
+private class IteratorImpl extends Iterator {
+  IteratorImpl() {
     this instanceof IteratorByTypedefs or
     exists(IteratorTraits it | it.getIteratorType() = this) or
     this instanceof StdIterator
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/StdContainer.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/StdContainer.qll
index b49a4be0bd35..708bed2855a5 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/StdContainer.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/StdContainer.qll
@@ -3,7 +3,7 @@
  */
 
 import semmle.code.cpp.models.interfaces.Taint
-import semmle.code.cpp.models.implementations.Iterator
+import semmle.code.cpp.models.interfaces.Iterator
 
 /**
  * Additional model for standard container constructors that reference the
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/StdMap.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/StdMap.qll
index 4c54a4988e5a..1a2c45d936e6 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/StdMap.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/StdMap.qll
@@ -3,7 +3,7 @@
  */
 
 import semmle.code.cpp.models.interfaces.Taint
-import semmle.code.cpp.models.implementations.Iterator
+import semmle.code.cpp.models.interfaces.Iterator
 
 /**
  * Additional model for map constructors using iterator inputs.
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/StdSet.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/StdSet.qll
index d18258e86b0c..acca0ca081f5 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/StdSet.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/StdSet.qll
@@ -3,7 +3,7 @@
  */
 
 import semmle.code.cpp.models.interfaces.Taint
-import semmle.code.cpp.models.implementations.Iterator
+import semmle.code.cpp.models.interfaces.Iterator
 
 /**
  * Additional model for set constructors using iterator inputs.
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll
index 62a19d8cb924..5082083999d0 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll
@@ -5,7 +5,8 @@
  */
 
 import semmle.code.cpp.models.interfaces.Taint
-import semmle.code.cpp.models.implementations.Iterator
+import semmle.code.cpp.models.interfaces.Iterator
+import semmle.code.cpp.models.interfaces.DataFlow
 
 /**
  * The `std::basic_string` template class.
diff --git a/cpp/ql/src/semmle/code/cpp/models/interfaces/Iterator.qll b/cpp/ql/src/semmle/code/cpp/models/interfaces/Iterator.qll
index 469d555a6352..bdace548355b 100644
--- a/cpp/ql/src/semmle/code/cpp/models/interfaces/Iterator.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/interfaces/Iterator.qll
@@ -26,3 +26,9 @@ abstract class GetIteratorFunction extends Function {
    */
   abstract predicate getsIterator(FunctionInput input, FunctionOutput output);
 }
+
+/**
+ * A type which can be used as an iterator.
+ */
+abstract class Iterator extends Type {
+}

From e065466180b0c24d701b5cf924a9975053f69fde Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 6 Nov 2020 09:28:52 +0000
Subject: [PATCH 04/15] C++: Give Snprintf a proper interface.

---
 .../code/cpp/models/implementations/Printf.qll     | 14 ++++----------
 .../cpp/models/interfaces/FormattingFunction.qll   | 13 +++++++++++++
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll
index 50b9bac576c1..cb808d4b443d 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll
@@ -119,11 +119,10 @@ private class Sprintf extends FormattingFunction {
 }
 
 /**
- * The standard functions `snprintf` and `swprintf`, and their
- * Microsoft and glib variants.
+ * Implements `Snprintf`.
  */
-private class Snprintf extends FormattingFunction {
-  Snprintf() {
+private class SnprintfImpl extends Snprintf {
+  SnprintfImpl() {
     this instanceof TopLevelFunction and
     (
       hasGlobalOrStdName("snprintf") or // C99 defines snprintf
@@ -180,12 +179,7 @@ private class Snprintf extends FormattingFunction {
     )
   }
 
-  /**
-   * Holds if this function returns the length of the formatted string
-   * that would have been output, regardless of the amount of space
-   * in the buffer.
-   */
-  predicate returnsFullFormatLength() {
+  override predicate returnsFullFormatLength() {
     (
       hasGlobalOrStdName("snprintf") or
       hasGlobalName("g_snprintf") or
diff --git a/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll b/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll
index f97646ca8330..a8c2923aa900 100644
--- a/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll
@@ -165,3 +165,16 @@ abstract class FormattingFunction extends ArrayFunction, TaintFunction {
     )
   }
 }
+
+/**
+ * The standard functions `snprintf` and `swprintf`, and their
+ * Microsoft and glib variants.
+ */
+abstract class Snprintf extends FormattingFunction {
+  /**
+   * Holds if this function returns the length of the formatted string
+   * that would have been output, regardless of the amount of space
+   * in the buffer.
+   */
+  predicate returnsFullFormatLength() { none() }
+}
\ No newline at end of file

From 74a4f5887b161fef0faa0f181b710c886feebbcb Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Thu, 5 Nov 2020 11:11:44 +0000
Subject: [PATCH 05/15] C++: Remove implementation import from printf.qll.

---
 cpp/ql/src/semmle/code/cpp/commons/Printf.qll              | 1 -
 cpp/ql/src/semmle/code/cpp/models/implementations/Inet.qll | 1 +
 cpp/ql/src/semmle/code/cpp/security/CommandExecution.qll   | 1 +
 cpp/ql/src/semmle/code/cpp/security/FileWrite.qll          | 1 +
 4 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/cpp/ql/src/semmle/code/cpp/commons/Printf.qll b/cpp/ql/src/semmle/code/cpp/commons/Printf.qll
index e441dd66adcc..53376acd85d7 100644
--- a/cpp/ql/src/semmle/code/cpp/commons/Printf.qll
+++ b/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__"] }
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Inet.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Inet.qll
index 736945468ede..397dca69fed1 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Inet.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Inet.qll
@@ -1,4 +1,5 @@
 import semmle.code.cpp.models.interfaces.Taint
+import semmle.code.cpp.models.interfaces.Alias
 import semmle.code.cpp.models.interfaces.ArrayFunction
 
 private class InetNtoa extends TaintFunction {
diff --git a/cpp/ql/src/semmle/code/cpp/security/CommandExecution.qll b/cpp/ql/src/semmle/code/cpp/security/CommandExecution.qll
index 5a24184e1a22..38187f2e0e6e 100644
--- a/cpp/ql/src/semmle/code/cpp/security/CommandExecution.qll
+++ b/cpp/ql/src/semmle/code/cpp/security/CommandExecution.qll
@@ -3,6 +3,7 @@
 import cpp
 import semmle.code.cpp.security.FunctionWithWrappers
 import semmle.code.cpp.models.interfaces.SideEffect
+import semmle.code.cpp.models.interfaces.Alias
 
 /**
  * A function for running a command using a command interpreter.
diff --git a/cpp/ql/src/semmle/code/cpp/security/FileWrite.qll b/cpp/ql/src/semmle/code/cpp/security/FileWrite.qll
index 9f63ce99c0b8..22c39b0b2c4c 100644
--- a/cpp/ql/src/semmle/code/cpp/security/FileWrite.qll
+++ b/cpp/ql/src/semmle/code/cpp/security/FileWrite.qll
@@ -3,6 +3,7 @@
  */
 
 import cpp
+import semmle.code.cpp.models.implementations.Printf
 
 /**
  * A function call that writes to a file.

From 62a8427d37ac2eff62b5f018a9840fd930784fd6 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Thu, 5 Nov 2020 16:46:46 +0000
Subject: [PATCH 06/15] C++: Change note.

---
 cpp/change-notes/2020-11-05-private-models.md | 3 +++
 1 file changed, 3 insertions(+)
 create mode 100644 cpp/change-notes/2020-11-05-private-models.md

diff --git a/cpp/change-notes/2020-11-05-private-models.md b/cpp/change-notes/2020-11-05-private-models.md
new file mode 100644
index 000000000000..7a2addf973d8
--- /dev/null
+++ b/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`.

From 2c7a01952e7993addd95ed65fca53ba1a7c466c2 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Tue, 10 Nov 2020 15:30:47 +0000
Subject: [PATCH 07/15] C++: Improve the changes to Iterator.

---
 .../code/cpp/models/implementations/Iterator.qll  | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll
index 804a812fe442..a940120caac1 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll
@@ -29,7 +29,7 @@ private class IteratorTraits extends Class {
 /**
  * A type which has the typedefs expected for an iterator.
  */
-private class IteratorByTypedefs extends Class {
+private class IteratorByTypedefs extends Iterator, Class {
   IteratorByTypedefs() {
     this.getAMember().(TypedefType).hasName("difference_type") and
     this.getAMember().(TypedefType).hasName("value_type") and
@@ -43,18 +43,17 @@ private class IteratorByTypedefs extends Class {
 /**
  * The `std::iterator` class.
  */
-private class StdIterator extends Class {
+private class StdIterator extends Iterator, Class {
   StdIterator() { this.hasQualifiedName("std", "iterator") }
 }
 
 /**
- * Implements `Iterator`.
+ * A type that is deduced to be an iterator because there is a corresponding
+ * `std::iterator_traits` instantiation for it.
  */
-private class IteratorImpl extends Iterator {
-  IteratorImpl() {
-    this instanceof IteratorByTypedefs or
-    exists(IteratorTraits it | it.getIteratorType() = this) or
-    this instanceof StdIterator
+private class IteratorByTraits extends Iterator {
+  IteratorByTraits() {
+    exists(IteratorTraits it | it.getIteratorType() = this)
   }
 }
 

From 5359e134213d9b8b5dd14610a24395a1cbfcdf69 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Wed, 11 Nov 2020 15:35:34 +0000
Subject: [PATCH 08/15] C++: Remove abstraction of
 OperatorNew/DeleteAllocationFunction.

---
 .../cpp/models/implementations/Allocation.qll | 28 -------------------
 .../models/implementations/Deallocation.qll   | 20 -------------
 .../code/cpp/models/interfaces/Allocation.qll | 25 +++++++++++++++--
 .../cpp/models/interfaces/Deallocation.qll    | 16 ++++++++++-
 4 files changed, 38 insertions(+), 51 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll
index 008d0b4c8aa9..709d3c3db004 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll
@@ -236,34 +236,6 @@ private class SizelessAllocationFunction extends AllocationFunction {
   }
 }
 
-/**
- * Implements `OperatorNewAllocationFunction`.
- */
-private class OperatorNewAllocationFunctionImpl extends OperatorNewAllocationFunction {
-  OperatorNewAllocationFunctionImpl() {
-    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()) }
-
-  override 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
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll
index 7604c630f8b1..3c65aa699be6 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll
@@ -89,26 +89,6 @@ private class StandardDeallocationFunction extends DeallocationFunction {
   override int getFreedArg() { result = freedArg }
 }
 
-/**
- * Implements `OperatorDeleteDeallocationFunction`.
- */
-private class OperatorDeleteDeallocationFunctionImpl extends OperatorDeleteDeallocationFunction {
-  OperatorDeleteDeallocationFunctionImpl() {
-    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`.
  */
diff --git a/cpp/ql/src/semmle/code/cpp/models/interfaces/Allocation.qll b/cpp/ql/src/semmle/code/cpp/models/interfaces/Allocation.qll
index 60671dc155f2..65c677761cc8 100644
--- a/cpp/ql/src/semmle/code/cpp/models/interfaces/Allocation.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/interfaces/Allocation.qll
@@ -91,10 +91,31 @@ abstract class AllocationExpr extends Expr {
  * `new` or `new[]` expressions.  Note that `new` and `new[]` are not function
  * calls, but these functions may also be called directly.
  */
-abstract class OperatorNewAllocationFunction extends AllocationFunction {
+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() { none()}
+  int getPlacementArgument() {
+    getNumberOfParameters() = 2 and
+    getParameter(1).getType() instanceof VoidPointerType and
+    result = 1
+  }
 }
diff --git a/cpp/ql/src/semmle/code/cpp/models/interfaces/Deallocation.qll b/cpp/ql/src/semmle/code/cpp/models/interfaces/Deallocation.qll
index 36efd0f9a234..501839de6781 100644
--- a/cpp/ql/src/semmle/code/cpp/models/interfaces/Deallocation.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/interfaces/Deallocation.qll
@@ -36,5 +36,19 @@ abstract class DeallocationExpr extends Expr {
  * with `delete` or `delete[]` expressions.  Note that `delete` and `delete[]`
  * are not function calls, but these functions may also be called directly.
  */
-abstract class OperatorDeleteDeallocationFunction extends DeallocationFunction {
+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 }
 }

From 0804df42d1411019f18159ab44e98e9cf7f0d800 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Thu, 12 Nov 2020 18:23:11 +0000
Subject: [PATCH 09/15] C++: Autoformat.

---
 cpp/ql/src/semmle/code/cpp/models/implementations/GetDelim.qll | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/GetDelim.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/GetDelim.qll
index 57fac090f57c..3561caee7fb2 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/GetDelim.qll
+++ b/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`.
  */
-private 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) {

From dfcb0ae7c219c06f37688b07cfe8a73ce1f9823e Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 13 Nov 2020 14:39:33 +0000
Subject: [PATCH 10/15] C++: Autoformat.

---
 cpp/ql/src/semmle/code/cpp/models/implementations/Memset.qll | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Memset.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Memset.qll
index 2fba6cfaf96c..0436fbeead99 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Memset.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Memset.qll
@@ -12,7 +12,8 @@ import semmle.code.cpp.models.interfaces.SideEffect
 /**
  * The standard function `memset` and its assorted variants
  */
-private class MemsetFunction extends ArrayFunction, DataFlowFunction, AliasFunction, SideEffectFunction {
+private class MemsetFunction extends ArrayFunction, DataFlowFunction, AliasFunction,
+  SideEffectFunction {
   MemsetFunction() {
     hasGlobalName("memset") or
     hasGlobalName("wmemset") or

From 4b8f33813983adfad1c739b4be55fd6968af78d6 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Mon, 16 Nov 2020 10:14:14 +0000
Subject: [PATCH 11/15] C++: Autoformat.

---
 .../cpp/models/implementations/Iterator.qll   | 19 ++++++-------
 .../code/cpp/models/implementations/Pure.qll  | 11 ++++----
 .../cpp/models/implementations/Strcpy.qll     | 28 ++++++++-----------
 .../models/interfaces/FormattingFunction.qll  |  2 +-
 .../code/cpp/models/interfaces/Iterator.qll   |  3 +-
 5 files changed, 28 insertions(+), 35 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll
index a940120caac1..bd063a29e970 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll
@@ -52,9 +52,7 @@ private class StdIterator extends Iterator, Class {
  * `std::iterator_traits` instantiation for it.
  */
 private class IteratorByTraits extends Iterator {
-  IteratorByTraits() {
-    exists(IteratorTraits it | it.getIteratorType() = this)
-  }
+  IteratorByTraits() { exists(IteratorTraits it | it.getIteratorType() = this) }
 }
 
 private FunctionInput getIteratorArgumentInput(Operator op, int index) {
@@ -80,7 +78,8 @@ private FunctionInput getIteratorArgumentInput(Operator op, int index) {
 /**
  * A non-member prefix `operator*` function for an iterator type.
  */
-private class IteratorPointerDereferenceOperator extends Operator, TaintFunction, IteratorReferenceFunction {
+private class IteratorPointerDereferenceOperator extends Operator, TaintFunction,
+  IteratorReferenceFunction {
   FunctionInput iteratorInput;
 
   IteratorPointerDereferenceOperator() {
@@ -244,7 +243,8 @@ private class IteratorBinaryArithmeticMemberOperator extends MemberFunction, Tai
 /**
  * An `operator+=` or `operator-=` member function of an iterator class.
  */
-private class IteratorAssignArithmeticMemberOperator extends MemberFunction, DataFlowFunction, TaintFunction {
+private class IteratorAssignArithmeticMemberOperator extends MemberFunction, DataFlowFunction,
+  TaintFunction {
   IteratorAssignArithmeticMemberOperator() {
     this.hasName(["operator+=", "operator-="]) and
     this.getDeclaringType() instanceof Iterator
@@ -267,7 +267,8 @@ private class IteratorAssignArithmeticMemberOperator extends MemberFunction, Dat
 /**
  * An `operator[]` member function of an iterator class.
  */
-private class IteratorArrayMemberOperator extends MemberFunction, TaintFunction, IteratorReferenceFunction {
+private class IteratorArrayMemberOperator extends MemberFunction, TaintFunction,
+  IteratorReferenceFunction {
   IteratorArrayMemberOperator() {
     this.hasName("operator[]") and
     this.getDeclaringType() instanceof Iterator
@@ -307,10 +308,8 @@ private class IteratorAssignmentMemberOperator extends MemberFunction, TaintFunc
 private class BeginOrEndFunction extends MemberFunction, TaintFunction, GetIteratorFunction {
   BeginOrEndFunction() {
     this
-        .hasName([
-            "begin", "cbegin", "rbegin", "crbegin", "end", "cend", "rend", "crend", "before_begin",
-            "cbefore_begin"
-          ]) and
+        .hasName(["begin", "cbegin", "rbegin", "crbegin", "end", "cend", "rend", "crend",
+              "before_begin", "cbefore_begin"]) and
     this.getType().getUnspecifiedType() instanceof Iterator
   }
 
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll
index 44ba87b4f732..8ef762e416ca 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll
@@ -3,13 +3,12 @@ import semmle.code.cpp.models.interfaces.Taint
 import semmle.code.cpp.models.interfaces.Alias
 import semmle.code.cpp.models.interfaces.SideEffect
 
-private class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunction, SideEffectFunction {
+private class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunction,
+  SideEffectFunction {
   PureStrFunction() {
-    hasGlobalOrStdName([
-        "atof", "atoi", "atol", "atoll", "strcasestr", "strchnul", "strchr", "strchrnul", "strstr",
-        "strpbrk", "strcmp", "strcspn", "strncmp", "strrchr", "strspn", "strtod", "strtof",
-        "strtol", "strtoll", "strtoq", "strtoul"
-      ])
+    hasGlobalOrStdName(["atof", "atoi", "atol", "atoll", "strcasestr", "strchnul", "strchr",
+          "strchrnul", "strstr", "strpbrk", "strcmp", "strcspn", "strncmp", "strrchr", "strspn",
+          "strtod", "strtof", "strtol", "strtoll", "strtoq", "strtoul"])
   }
 
   override predicate hasArrayInput(int bufParam) {
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Strcpy.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Strcpy.qll
index c1268bf2460e..d0b3f92fa0fd 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Strcpy.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Strcpy.qll
@@ -14,24 +14,20 @@ import semmle.code.cpp.models.interfaces.SideEffect
 class StrcpyFunction extends ArrayFunction, DataFlowFunction, TaintFunction, SideEffectFunction {
   StrcpyFunction() {
     getName() =
-      [
-        "strcpy", // strcpy(dst, src)
-        "wcscpy", // wcscpy(dst, src)
-        "_mbscpy", // _mbscpy(dst, src)
-        "strncpy", // strncpy(dst, src, max_amount)
-        "_strncpy_l", // _strncpy_l(dst, src, max_amount, locale)
-        "wcsncpy", // wcsncpy(dst, src, max_amount)
-        "_wcsncpy_l", // _wcsncpy_l(dst, src, max_amount, locale)
-        "_mbsncpy", // _mbsncpy(dst, src, max_amount)
-        "_mbsncpy_l" // _mbsncpy_l(dst, src, max_amount, locale)
-      ]
+      ["strcpy", // strcpy(dst, src)
+          "wcscpy", // wcscpy(dst, src)
+          "_mbscpy", // _mbscpy(dst, src)
+          "strncpy", // strncpy(dst, src, max_amount)
+          "_strncpy_l", // _strncpy_l(dst, src, max_amount, locale)
+          "wcsncpy", // wcsncpy(dst, src, max_amount)
+          "_wcsncpy_l", // _wcsncpy_l(dst, src, max_amount, locale)
+          "_mbsncpy", // _mbsncpy(dst, src, max_amount)
+          "_mbsncpy_l"] // _mbsncpy_l(dst, src, max_amount, locale)
     or
     getName() =
-      [
-        "strcpy_s", // strcpy_s(dst, max_amount, src)
-        "wcscpy_s", // wcscpy_s(dst, max_amount, src)
-        "_mbscpy_s" // _mbscpy_s(dst, max_amount, src)
-      ] and
+      ["strcpy_s", // strcpy_s(dst, max_amount, src)
+          "wcscpy_s", // wcscpy_s(dst, max_amount, src)
+          "_mbscpy_s"] and // _mbscpy_s(dst, max_amount, src)
     // exclude the 2-parameter template versions
     // that find the size of a fixed size destination buffer.
     getNumberOfParameters() = 3
diff --git a/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll b/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll
index a8c2923aa900..c22a9d89e537 100644
--- a/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll
@@ -177,4 +177,4 @@ abstract class Snprintf extends FormattingFunction {
    * in the buffer.
    */
   predicate returnsFullFormatLength() { none() }
-}
\ No newline at end of file
+}
diff --git a/cpp/ql/src/semmle/code/cpp/models/interfaces/Iterator.qll b/cpp/ql/src/semmle/code/cpp/models/interfaces/Iterator.qll
index 548d539566e6..9a260a332557 100644
--- a/cpp/ql/src/semmle/code/cpp/models/interfaces/Iterator.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/interfaces/Iterator.qll
@@ -30,5 +30,4 @@ abstract class GetIteratorFunction extends Function {
 /**
  * A type which can be used as an iterator.
  */
-abstract class Iterator extends Type {
-}
+abstract class Iterator extends Type { }

From fddd3531552811c8a934ea42ec7b1de0e2807b54 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 20 Nov 2020 20:15:45 +0000
Subject: [PATCH 12/15] C++: Updated autoformat.

---
 .../cpp/models/implementations/Iterator.qll   |  6 ++--
 .../code/cpp/models/implementations/Pure.qll  |  8 ++++--
 .../cpp/models/implementations/Strcpy.qll     | 28 +++++++++++--------
 3 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll
index bd063a29e970..60f95bf1a418 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll
@@ -308,8 +308,10 @@ private class IteratorAssignmentMemberOperator extends MemberFunction, TaintFunc
 private class BeginOrEndFunction extends MemberFunction, TaintFunction, GetIteratorFunction {
   BeginOrEndFunction() {
     this
-        .hasName(["begin", "cbegin", "rbegin", "crbegin", "end", "cend", "rend", "crend",
-              "before_begin", "cbefore_begin"]) and
+        .hasName([
+            "begin", "cbegin", "rbegin", "crbegin", "end", "cend", "rend", "crend", "before_begin",
+            "cbefore_begin"
+          ]) and
     this.getType().getUnspecifiedType() instanceof Iterator
   }
 
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll
index 8ef762e416ca..2abf99ee33b6 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll
@@ -6,9 +6,11 @@ import semmle.code.cpp.models.interfaces.SideEffect
 private class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunction,
   SideEffectFunction {
   PureStrFunction() {
-    hasGlobalOrStdName(["atof", "atoi", "atol", "atoll", "strcasestr", "strchnul", "strchr",
-          "strchrnul", "strstr", "strpbrk", "strcmp", "strcspn", "strncmp", "strrchr", "strspn",
-          "strtod", "strtof", "strtol", "strtoll", "strtoq", "strtoul"])
+    hasGlobalOrStdName([
+        "atof", "atoi", "atol", "atoll", "strcasestr", "strchnul", "strchr", "strchrnul", "strstr",
+        "strpbrk", "strcmp", "strcspn", "strncmp", "strrchr", "strspn", "strtod", "strtof",
+        "strtol", "strtoll", "strtoq", "strtoul"
+      ])
   }
 
   override predicate hasArrayInput(int bufParam) {
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Strcpy.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Strcpy.qll
index d0b3f92fa0fd..061209d65b79 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Strcpy.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Strcpy.qll
@@ -14,20 +14,24 @@ import semmle.code.cpp.models.interfaces.SideEffect
 class StrcpyFunction extends ArrayFunction, DataFlowFunction, TaintFunction, SideEffectFunction {
   StrcpyFunction() {
     getName() =
-      ["strcpy", // strcpy(dst, src)
-          "wcscpy", // wcscpy(dst, src)
-          "_mbscpy", // _mbscpy(dst, src)
-          "strncpy", // strncpy(dst, src, max_amount)
-          "_strncpy_l", // _strncpy_l(dst, src, max_amount, locale)
-          "wcsncpy", // wcsncpy(dst, src, max_amount)
-          "_wcsncpy_l", // _wcsncpy_l(dst, src, max_amount, locale)
-          "_mbsncpy", // _mbsncpy(dst, src, max_amount)
-          "_mbsncpy_l"] // _mbsncpy_l(dst, src, max_amount, locale)
+      [
+        "strcpy", // strcpy(dst, src)
+        "wcscpy", // wcscpy(dst, src)
+        "_mbscpy", // _mbscpy(dst, src)
+        "strncpy", // strncpy(dst, src, max_amount)
+        "_strncpy_l", // _strncpy_l(dst, src, max_amount, locale)
+        "wcsncpy", // wcsncpy(dst, src, max_amount)
+        "_wcsncpy_l", // _wcsncpy_l(dst, src, max_amount, locale)
+        "_mbsncpy", // _mbsncpy(dst, src, max_amount)
+        "_mbsncpy_l"
+      ] // _mbsncpy_l(dst, src, max_amount, locale)
     or
     getName() =
-      ["strcpy_s", // strcpy_s(dst, max_amount, src)
-          "wcscpy_s", // wcscpy_s(dst, max_amount, src)
-          "_mbscpy_s"] and // _mbscpy_s(dst, max_amount, src)
+      [
+        "strcpy_s", // strcpy_s(dst, max_amount, src)
+        "wcscpy_s", // wcscpy_s(dst, max_amount, src)
+        "_mbscpy_s"
+      ] and // _mbscpy_s(dst, max_amount, src)
     // exclude the 2-parameter template versions
     // that find the size of a fixed size destination buffer.
     getNumberOfParameters() = 3

From 7015a9cf5301d23b94ba638de9286114144d33d8 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Mon, 23 Nov 2020 14:47:22 +0000
Subject: [PATCH 13/15] C++: Un-private a few classes that are now used by the
 current DefaultSafeExternalAPIFunction implementation.

---
 .../CWE/CWE-020/SafeExternalAPIFunction.qll   |  1 +
 .../code/cpp/models/implementations/Pure.qll  | 30 ++++++++++++-------
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/cpp/ql/src/Security/CWE/CWE-020/SafeExternalAPIFunction.qll b/cpp/ql/src/Security/CWE/CWE-020/SafeExternalAPIFunction.qll
index cdcebe6884c1..764b04d76db2 100644
--- a/cpp/ql/src/Security/CWE/CWE-020/SafeExternalAPIFunction.qll
+++ b/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
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll
index 46111e84d219..16cec0272488 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll
@@ -3,14 +3,16 @@ import semmle.code.cpp.models.interfaces.Taint
 import semmle.code.cpp.models.interfaces.Alias
 import semmle.code.cpp.models.interfaces.SideEffect
 
-/** Pure string functions. */
-private class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunction, SideEffectFunction {
+/**
+ * Pure string functions.
+ *
+ * INTERNAL: do not use.
+ */
+class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunction, SideEffectFunction {
   PureStrFunction() {
-    hasGlobalOrStdName([
-        "atof", "atoi", "atol", "atoll", "strcasestr", "strchnul", "strchr", "strchrnul", "strstr",
-        "strpbrk", "strcmp", "strcspn", "strncmp", "strrchr", "strspn", "strtod", "strtof",
-        "strtol", "strtoll", "strtoq", "strtoul"
-      ])
+    hasGlobalOrStdName(["atof", "atoi", "atol", "atoll", "strcasestr", "strchnul", "strchr",
+          "strchrnul", "strstr", "strpbrk", "strcmp", "strcspn", "strncmp", "strrchr", "strspn",
+          "strtod", "strtof", "strtol", "strtoll", "strtoq", "strtoul"])
   }
 
   override predicate hasArrayInput(int bufParam) {
@@ -59,8 +61,12 @@ private class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunctio
   }
 }
 
-/** String standard `strlen` function, and related functions for computing string lengths. */
-private class StrLenFunction extends AliasFunction, ArrayFunction, SideEffectFunction {
+/**
+ * String standard `strlen` function, and related functions for computing string lengths.
+ *
+ * INTERNAL: do not use.
+ */
+class StrLenFunction extends AliasFunction, ArrayFunction, SideEffectFunction {
   StrLenFunction() {
     hasGlobalOrStdName(["strlen", "strnlen", "wcslen"])
     or
@@ -110,7 +116,11 @@ private class PureFunction extends TaintFunction, SideEffectFunction {
   override predicate hasOnlySpecificWriteSideEffects() { any() }
 }
 
-/** Pure raw-memory functions. */
+/**
+ * Pure raw-memory functions.
+ *
+ * INTERNAL: do not use.
+ */
 class PureMemFunction extends AliasFunction, ArrayFunction, TaintFunction, SideEffectFunction {
   PureMemFunction() { hasGlobalOrStdName(["memchr", "memrchr", "rawmemchr", "memcmp", "memmem"]) }
 

From 8184f76d1ff051b885d4e008f92e2f3c6a5ee345 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Tue, 24 Nov 2020 16:29:14 +0000
Subject: [PATCH 14/15] C++: Sync identical files.

---
 cpp/ql/src/Security/CWE/CWE-020/ir/SafeExternalAPIFunction.qll | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cpp/ql/src/Security/CWE/CWE-020/ir/SafeExternalAPIFunction.qll b/cpp/ql/src/Security/CWE/CWE-020/ir/SafeExternalAPIFunction.qll
index cdcebe6884c1..764b04d76db2 100644
--- a/cpp/ql/src/Security/CWE/CWE-020/ir/SafeExternalAPIFunction.qll
+++ b/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

From 71a8ac518394233e3affe5645c543b72fee2b6b9 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Tue, 24 Nov 2020 18:41:05 +0000
Subject: [PATCH 15/15] C++: Autoformat.

---
 .../src/semmle/code/cpp/models/implementations/Pure.qll   | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll
index 16cec0272488..39eb8f63c24d 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll
@@ -10,9 +10,11 @@ import semmle.code.cpp.models.interfaces.SideEffect
  */
 class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunction, SideEffectFunction {
   PureStrFunction() {
-    hasGlobalOrStdName(["atof", "atoi", "atol", "atoll", "strcasestr", "strchnul", "strchr",
-          "strchrnul", "strstr", "strpbrk", "strcmp", "strcspn", "strncmp", "strrchr", "strspn",
-          "strtod", "strtof", "strtol", "strtoll", "strtoq", "strtoul"])
+    hasGlobalOrStdName([
+        "atof", "atoi", "atol", "atoll", "strcasestr", "strchnul", "strchr", "strchrnul", "strstr",
+        "strpbrk", "strcmp", "strcspn", "strncmp", "strrchr", "strspn", "strtod", "strtof",
+        "strtol", "strtoll", "strtoq", "strtoul"
+      ])
   }
 
   override predicate hasArrayInput(int bufParam) {