From 399ac1f11297d24231ee8daab34daafd7618db0f Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Wed, 6 Nov 2019 15:57:44 +0000
Subject: [PATCH 1/6] CPP: Rename 'getAssertedFalseCondition' to something less
 misleading.

---
 .../Security/CWE/CWE-457/InitializationFunctions.qll   | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/cpp/ql/src/Security/CWE/CWE-457/InitializationFunctions.qll b/cpp/ql/src/Security/CWE/CWE-457/InitializationFunctions.qll
index 240bd7aa25e3..1c299b9ad517 100644
--- a/cpp/ql/src/Security/CWE/CWE-457/InitializationFunctions.qll
+++ b/cpp/ql/src/Security/CWE/CWE-457/InitializationFunctions.qll
@@ -89,9 +89,9 @@ class ParameterNullCheck extends ParameterCheck {
       (
         va = this.(NotExpr).getOperand() or
         va = any(EQExpr eq | eq = this and eq.getAnOperand().getValue() = "0").getAnOperand() or
-        va = getAssertedFalseCondition(this) or
+        va = getCheckedFalseCondition(this) or
         va = any(NEExpr eq |
-            eq = getAssertedFalseCondition(this) and eq.getAnOperand().getValue() = "0"
+            eq = getCheckedFalseCondition(this) and eq.getAnOperand().getValue() = "0"
           ).getAnOperand()
       )
       or
@@ -101,7 +101,7 @@ class ParameterNullCheck extends ParameterCheck {
         va = this or
         va = any(NEExpr eq | eq = this and eq.getAnOperand().getValue() = "0").getAnOperand() or
         va = any(EQExpr eq |
-            eq = getAssertedFalseCondition(this) and eq.getAnOperand().getValue() = "0"
+            eq = getCheckedFalseCondition(this) and eq.getAnOperand().getValue() = "0"
           ).getAnOperand()
       )
     )
@@ -669,7 +669,7 @@ FieldAccess getAFieldAccess(Variable v) {
 }
 
 /**
- * Gets a condition which is asserted to be false by the given `ne` expression, according to this pattern:
+ * Gets a condition which is checked to be false by the given `ne` expression, according to this pattern:
  * ```
  * int a = !!result;
  * if (!a) {  // <- ne
@@ -677,7 +677,7 @@ FieldAccess getAFieldAccess(Variable v) {
  * }
  * ```
  */
-Expr getAssertedFalseCondition(NotExpr ne) {
+private Expr getCheckedFalseCondition(NotExpr ne) {
   exists(LocalVariable v |
     result = v.getInitializer().getExpr().(NotExpr).getOperand().(NotExpr).getOperand() and
     ne.getOperand() = v.getAnAccess() and

From 0c3f4e530f62ce1340c022948f5a2a84108e6353 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Wed, 6 Nov 2019 16:07:28 +0000
Subject: [PATCH 2/6] CPP: Make some library predicates private.

---
 .../src/Security/CWE/CWE-457/InitializationFunctions.qll  | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/cpp/ql/src/Security/CWE/CWE-457/InitializationFunctions.qll b/cpp/ql/src/Security/CWE/CWE-457/InitializationFunctions.qll
index 1c299b9ad517..276b6043901e 100644
--- a/cpp/ql/src/Security/CWE/CWE-457/InitializationFunctions.qll
+++ b/cpp/ql/src/Security/CWE/CWE-457/InitializationFunctions.qll
@@ -567,7 +567,7 @@ Expr getAnInitializedArgument(Call call) { result = call.getArgument(initialized
  * the call, under the given context and evidence.
  */
 pragma[nomagic]
-int conditionallyInitializedArgument(
+private int conditionallyInitializedArgument(
   Call call, ConditionalInitializationFunction target, Context c, Evidence e
 ) {
   target = getTarget(call) and
@@ -588,7 +588,7 @@ Expr getAConditionallyInitializedArgument(
 /**
  * Gets the type signature for the functions parameters.
  */
-string typeSig(Function f) {
+private string typeSig(Function f) {
   result = concat(int i, Type pt |
       pt = f.getParameter(i).getType()
     |
@@ -599,7 +599,7 @@ string typeSig(Function f) {
 /**
  * Holds where qualifiedName and typeSig make up the signature for the function.
  */
-predicate functionSignature(Function f, string qualifiedName, string typeSig) {
+private predicate functionSignature(Function f, string qualifiedName, string typeSig) {
   qualifiedName = f.getQualifiedName() and
   typeSig = typeSig(f)
 }
@@ -611,7 +611,7 @@ predicate functionSignature(Function f, string qualifiedName, string typeSig) {
  * This is useful for identifying call to target dependencies across libraries, where the libraries
  * are never statically linked together.
  */
-Function getAPossibleDefinition(Function undefinedFunction) {
+private Function getAPossibleDefinition(Function undefinedFunction) {
   not undefinedFunction.isDefined() and
   exists(string qn, string typeSig |
     functionSignature(undefinedFunction, qn, typeSig) and functionSignature(result, qn, typeSig)

From 81c58d5a64cdda985da83f0395cca56230b9477c Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Wed, 6 Nov 2019 16:17:33 +0000
Subject: [PATCH 3/6] CPP: Improve QLDoc comments.

---
 cpp/ql/src/semmle/code/cpp/NestedFields.qll | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/NestedFields.qll b/cpp/ql/src/semmle/code/cpp/NestedFields.qll
index 12109be8ece1..c4be8b8b9ffd 100644
--- a/cpp/ql/src/semmle/code/cpp/NestedFields.qll
+++ b/cpp/ql/src/semmle/code/cpp/NestedFields.qll
@@ -1,9 +1,8 @@
 import cpp
 
 /**
- * Gets a `Field` that is nested within the given `Struct`.
- *
- * This identifies `Field`s which are located in the same memory
+ * Gets a `Field` that is within the given `Struct`, either directly or nested
+ * inside one or more levels of member structs.
  */
 private Field getANestedField(Struct s) {
   result = s.getAField()
@@ -15,7 +14,7 @@ private Field getANestedField(Struct s) {
 }
 
 /**
- * Unwraps a series of field accesses to determine the inner-most qualifier.
+ * Unwraps a series of field accesses to determine the outer-most qualifier.
  */
 private Expr getUltimateQualifier(FieldAccess fa) {
   exists(Expr qualifier | qualifier = fa.getQualifier() |

From e886cf729773f9e1f86ff35805e596e5f4274d9f Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Wed, 6 Nov 2019 16:27:06 +0000
Subject: [PATCH 4/6] CPP: 'i.e.' -> 'that is'.

---
 cpp/ql/src/Microsoft/SAL.qll                                   | 2 +-
 cpp/ql/src/semmle/code/cpp/dispatch/VirtualDispatch.qll        | 2 +-
 .../src/semmle/code/cpp/security/boostorg/asio/protocols.qll   | 3 ++-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/cpp/ql/src/Microsoft/SAL.qll b/cpp/ql/src/Microsoft/SAL.qll
index fe6b455fa9be..963a726ae54c 100644
--- a/cpp/ql/src/Microsoft/SAL.qll
+++ b/cpp/ql/src/Microsoft/SAL.qll
@@ -126,7 +126,7 @@ class SALParameter extends Parameter {
 }
 
 /**
- * A SAL element, i.e. a SAL annotation or a declaration entry
+ * A SAL element, that is, a SAL annotation or a declaration entry
  * that may have SAL annotations.
  */
 library class SALElement extends Element {
diff --git a/cpp/ql/src/semmle/code/cpp/dispatch/VirtualDispatch.qll b/cpp/ql/src/semmle/code/cpp/dispatch/VirtualDispatch.qll
index 2fa9322843e6..a181c9bb5430 100644
--- a/cpp/ql/src/semmle/code/cpp/dispatch/VirtualDispatch.qll
+++ b/cpp/ql/src/semmle/code/cpp/dispatch/VirtualDispatch.qll
@@ -63,7 +63,7 @@ module VirtualDispatch {
 
   /**
    * Holds if `c` cannot inherit the member function `f`,
-   * i.e. `c` or one of its supertypes overrides `f`.
+   * that is, `c` or one of its supertypes overrides `f`.
    */
   private predicate cannotInherit(Class c, MemberFunction f) {
     exists(Class overridingType, MemberFunction override |
diff --git a/cpp/ql/src/semmle/code/cpp/security/boostorg/asio/protocols.qll b/cpp/ql/src/semmle/code/cpp/security/boostorg/asio/protocols.qll
index ae3733473adc..af1a8eaf8e79 100644
--- a/cpp/ql/src/semmle/code/cpp/security/boostorg/asio/protocols.qll
+++ b/cpp/ql/src/semmle/code/cpp/security/boostorg/asio/protocols.qll
@@ -56,7 +56,8 @@ module BoostorgAsio {
     }
 
     /**
-     * returns the value for a approved protocols, but that are hard-coded (i.e. no protocol negotiation)
+     * returns the value for an approved protocol, but that are hard-coded
+     * (that is, no protocol negotiation)
      */
     EnumConstant getAnApprovedButHardcodedProtocolConstant() {
       result = this.getATls12ProtocolConstant()

From 6c38f55e28aa4b9ebff51ccddd6a414fa03f45de Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Wed, 6 Nov 2019 16:31:21 +0000
Subject: [PATCH 5/6] CPP: QLDoc protocols.qll.

---
 .../cpp/security/boostorg/asio/protocols.qll  | 61 ++++++++++---------
 1 file changed, 32 insertions(+), 29 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/security/boostorg/asio/protocols.qll b/cpp/ql/src/semmle/code/cpp/security/boostorg/asio/protocols.qll
index af1a8eaf8e79..73ccd49e2f8e 100644
--- a/cpp/ql/src/semmle/code/cpp/security/boostorg/asio/protocols.qll
+++ b/cpp/ql/src/semmle/code/cpp/security/boostorg/asio/protocols.qll
@@ -3,7 +3,7 @@ import semmle.code.cpp.dataflow.DataFlow
 
 module BoostorgAsio {
   /**
-   * Represents boost::asio::ssl::context enum
+   * Represents the `boost::asio::ssl::context` enum.
    */
   class SslContextMethod extends Enum {
     SslContextMethod() {
@@ -12,7 +12,7 @@ module BoostorgAsio {
     }
 
     /**
-     * returns the value for a banned protocol
+     * Gets an enumeration constant for a banned protocol.
      */
     EnumConstant getABannedProtocolConstant() {
       result = this.getAnEnumConstant() and
@@ -56,15 +56,15 @@ module BoostorgAsio {
     }
 
     /**
-     * returns the value for an approved protocol, but that are hard-coded
-     * (that is, no protocol negotiation)
+     * Gets an enumeration constant for an approved protocol, that is hard-coded
+     * (no protocol negotiation).
      */
     EnumConstant getAnApprovedButHardcodedProtocolConstant() {
       result = this.getATls12ProtocolConstant()
     }
 
     /**
-     * returns the value for a TLS v1.2 protocol
+     * Gets an enumeration constant for a TLS v1.2 protocol.
      */
     EnumConstant getATls12ProtocolConstant() {
       result = this.getAnEnumConstant() and
@@ -81,7 +81,7 @@ module BoostorgAsio {
     }
 
     /**
-     * returns the value for a TLS v1.3 protocol
+     * Gets an enumeration constant for a TLS v1.3 protocol.
      */
     EnumConstant getATls13ProtocolConstant() {
       result = this.getAnEnumConstant() and
@@ -98,7 +98,7 @@ module BoostorgAsio {
     }
 
     /**
-     * returns the value of a generic TLS or SSL/TLS protocol
+     * Gets an enumeration constant for a generic TLS or SSL/TLS protocol.
      */
     EnumConstant getAGenericTlsProtocolConstant() {
       result = this.getAnEnumConstant() and
@@ -117,7 +117,7 @@ module BoostorgAsio {
     }
 
     /**
-     * returns the value of a generic SSL/TLS protocol
+     * Gets an enumeration constant for a generic SSL/TLS protocol.
      */
     EnumConstant getASslv23ProtocolConstant() {
       result = this.getAnEnumConstant() and
@@ -136,7 +136,9 @@ module BoostorgAsio {
   }
 
   /**
-   * NOTE: ignore - Modern versions of OpenSSL do not support SSL v2 anymore, so this option is for backwards compatibility only
+   * Gets the value for the no_sslv2 constant, right shifted by 16 bits.
+   * 
+   * Note that modern versions of OpelSSL do not support SSL v2, so this option is for backwards compatibility only.
    */
   int getShiftedSslOptionsNoSsl2() {
     // SSL_OP_NO_SSLv2 was removed from modern OpenSSL versions
@@ -144,7 +146,7 @@ module BoostorgAsio {
   }
 
   /**
-   * RightShift(16) value for no_sslv3 constant
+   * Gets the value for the no_sslv3 constant, right shifted by 16 bits.
    */
   int getShiftedSslOptionsNoSsl3() {
     // SSL_OP_NO_SSLv3 == 0x02000000U
@@ -152,7 +154,7 @@ module BoostorgAsio {
   }
 
   /**
-   * RightShift(16) value for no_tlsv1 constant
+   * Gets the value for the no_tlsv1 constant, right shifted by 16 bits.
    */
   int getShiftedSslOptionsNoTls1() {
     // SSL_OP_NO_TLSv1 == 0x04000000U
@@ -160,7 +162,7 @@ module BoostorgAsio {
   }
 
   /**
-   * RightShift(16) value for no_tlsv1_1 constant
+   * Gets the value for the no_tlsv1_1 constant, right shifted by 16 bits.
    */
   int getShiftedSslOptionsNoTls1_1() {
     // SSL_OP_NO_TLSv1_1 == 0x10000000U
@@ -168,7 +170,7 @@ module BoostorgAsio {
   }
 
   /**
-   * RightShift(16) value for no_tlsv1_2 constant
+   * Gets the value for the no_tlsv1_2 constant, right shifted by 16 bits.
    */
   int getShiftedSslOptionsNoTls1_2() {
     // SSL_OP_NO_TLSv1_2 == 0x08000000U
@@ -176,7 +178,7 @@ module BoostorgAsio {
   }
 
   /**
-   * RightShift(16) value for no_tlsv1_3 constant
+   * Gets the value for the no_tlsv1_3 constant, right shifted by 16 bits.
    */
   int getShiftedSslOptionsNoTls1_3() {
     // SSL_OP_NO_TLSv1_2 == 0x20000000U
@@ -184,7 +186,7 @@ module BoostorgAsio {
   }
 
   /**
-   * Represents boost::asio::ssl::context class
+   * Represents the `boost::asio::ssl::context` class.
    */
   class SslContextClass extends Class {
     SslContextClass() { this.getQualifiedName() = "boost::asio::ssl::context" }
@@ -197,7 +199,7 @@ module BoostorgAsio {
   }
 
   /**
-   * Represents boost::asio::ssl::context::set_options member function
+   * Represents `boost::asio::ssl::context::set_options` member function.
    */
   class SslSetOptionsFunction extends Function {
     SslSetOptionsFunction() {
@@ -206,7 +208,7 @@ module BoostorgAsio {
   }
 
   /**
-   * holds if the expression represents a banned protocol
+   * Holds if the expression represents a banned protocol.
    */
   predicate isExprBannedBoostProtocol(Expr e) {
     exists(Literal va | va = e |
@@ -245,7 +247,7 @@ module BoostorgAsio {
   }
 
   /**
-   * holds if the expression represents a TLS v1.2 protocol
+   * Holds if the expression represents a TLS v1.2 protocol.
    */
   predicate isExprTls12BoostProtocol(Expr e) {
     exists(Literal va | va = e |
@@ -270,7 +272,7 @@ module BoostorgAsio {
   }
 
   /**
-   * holds if the expression represents a protocol that requires Crypto Board approval
+   * Holds if the expression represents a protocol that requires Crypto Board approval.
    */
   predicate isExprTls13BoostProtocol(Expr e) {
     exists(Literal va | va = e |
@@ -295,7 +297,7 @@ module BoostorgAsio {
   }
 
   /**
-   * holds if the expression represents a generic TLS or SSL/TLS protocol
+   * Holds if the expression represents a generic TLS or SSL/TLS protocol.
    */
   predicate isExprTlsBoostProtocol(Expr e) {
     exists(Literal va | va = e |
@@ -326,7 +328,7 @@ module BoostorgAsio {
   }
 
   /**
-   * holds if the expression represents a generic SSl/TLS protocol
+   * Holds if the expression represents a generic SSl/TLS protocol.
    */
   predicate isExprSslV23BoostProtocol(Expr e) {
     exists(Literal va | va = e |
@@ -352,7 +354,8 @@ module BoostorgAsio {
 
   //////////////////////// Dataflow /////////////////////
   /**
-   * Abstract - Protocol value Flows to the first argument of the context constructor
+   * Abstract class for flows of protocol values to the first argument of a context
+   * constructor.
    */
   abstract class SslContextCallAbstractConfig extends DataFlow::Configuration {
     bindingset[this]
@@ -367,7 +370,7 @@ module BoostorgAsio {
   }
 
   /**
-   * any Protocol value Flows to the first argument of the context constructor
+   * Any protocol value that flows to the first argument of a context constructor.
    */
   class SslContextCallConfig extends SslContextCallAbstractConfig {
     SslContextCallConfig() { this = "SslContextCallConfig" }
@@ -381,7 +384,7 @@ module BoostorgAsio {
   }
 
   /**
-   * a banned protocol value Flows to the first argument of the context constructor
+   * A banned protocol value that flows to the first argument of a context constructor.
    */
   class SslContextCallBannedProtocolConfig extends SslContextCallAbstractConfig {
     SslContextCallBannedProtocolConfig() { this = "SslContextCallBannedProtocolConfig" }
@@ -396,7 +399,7 @@ module BoostorgAsio {
   }
 
   /**
-   * a TLS 1.2 protocol value Flows to the first argument of the context constructor
+   * A TLS 1.2 protocol value that flows to the first argument of a context constructor.
    */
   class SslContextCallTls12ProtocolConfig extends SslContextCallAbstractConfig {
     SslContextCallTls12ProtocolConfig() { this = "SslContextCallTls12ProtocolConfig" }
@@ -411,7 +414,7 @@ module BoostorgAsio {
   }
 
   /**
-   * a TLS 1.3 protocol value Flows to the first argument of the context constructor
+   * A TLS 1.3 protocol value that flows to the first argument of a context constructor.
    */
   class SslContextCallTls13ProtocolConfig extends SslContextCallAbstractConfig {
     SslContextCallTls13ProtocolConfig() { this = "SslContextCallTls12ProtocolConfig" }
@@ -426,7 +429,7 @@ module BoostorgAsio {
   }
 
   /**
-   * a generic TLS protocol value Flows to the first argument of the context constructor
+   * A generic TLS protocol value that flows to the first argument of a context constructor.
    */
   class SslContextCallTlsProtocolConfig extends SslContextCallAbstractConfig {
     SslContextCallTlsProtocolConfig() { this = "SslContextCallTlsProtocolConfig" }
@@ -441,7 +444,7 @@ module BoostorgAsio {
   }
 
   /**
-   * a context constructor call flows to a call calling SetOptions()
+   * A context constructor call that flows to a call to `SetOptions()`.
    */
   class SslContextFlowsToSetOptionConfig extends DataFlow::Configuration {
     SslContextFlowsToSetOptionConfig() { this = "SslContextFlowsToSetOptionConfig" }
@@ -465,7 +468,7 @@ module BoostorgAsio {
   }
 
   /**
-   * an option value flows to the 1st parameter of SetOptions()
+   * An option value that flows to the first parameter of a call to `SetOptions()`.
    */
   class SslOptionConfig extends DataFlow::Configuration {
     SslOptionConfig() { this = "SslOptionConfig" }

From 58b6fc6bbff99cc97932e5d35ed54608ddb7f5bd Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 8 Nov 2019 16:06:23 +0000
Subject: [PATCH 6/6] CPP: Autoformat.

---
 cpp/ql/src/semmle/code/cpp/security/boostorg/asio/protocols.qll | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cpp/ql/src/semmle/code/cpp/security/boostorg/asio/protocols.qll b/cpp/ql/src/semmle/code/cpp/security/boostorg/asio/protocols.qll
index 73ccd49e2f8e..e113d5e5745b 100644
--- a/cpp/ql/src/semmle/code/cpp/security/boostorg/asio/protocols.qll
+++ b/cpp/ql/src/semmle/code/cpp/security/boostorg/asio/protocols.qll
@@ -137,7 +137,7 @@ module BoostorgAsio {
 
   /**
    * Gets the value for the no_sslv2 constant, right shifted by 16 bits.
-   * 
+   *
    * Note that modern versions of OpelSSL do not support SSL v2, so this option is for backwards compatibility only.
    */
   int getShiftedSslOptionsNoSsl2() {