Merge pull request #2274 from geoffw0/oddsends

CPP: Clean up new queries and libraries

Author: jbj

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 {

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()
       )
     )
@@ -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)
@@ -684,15 +684,15 @@ 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
  *   ....
  * }
  * ```
  */
-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

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() |

cpp/ql/src/semmle/code/cpp/dispatch/VirtualDispatch.qll

@@ -67,7 +67,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 |

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,14 +56,15 @@ module BoostorgAsio {
     }
 
     /**
-     * returns the value for a approved protocols, but that are hard-coded (i.e. 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
@@ -80,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
@@ -97,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
@@ -116,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
@@ -135,55 +136,57 @@ 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
     result = 0
   }
 
   /**
-   * 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
     result = 512
   }
 
   /**
-   * 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
     result = 1024
   }
 
   /**
-   * 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
     result = 4096
   }
 
   /**
-   * 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
     result = 2048
   }
 
   /**
-   * 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
     result = 8192
   }
 
   /**
-   * Represents boost::asio::ssl::context class
+   * Represents the `boost::asio::ssl::context` class.
    */
   class SslContextClass extends Class {
     SslContextClass() { this.getQualifiedName() = "boost::asio::ssl::context" }
@@ -196,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() {
@@ -205,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 |
@@ -244,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 |
@@ -269,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 |
@@ -294,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 |
@@ -325,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 |
@@ -351,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]
@@ -366,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" }
@@ -380,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" }
@@ -395,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" }
@@ -410,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" }
@@ -425,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" }
@@ -440,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" }
@@ -464,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" }