Merge branch 'main' into python-untrusted-flow

Author: RasmusWL

cpp/change-notes/2020-11-27-downgrade-to-recommendation.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The queries `cpp/local-variable-hides-global-variable` and `cpp/missing-header-guard` now have severity `recommendation` instead of `warning`.

cpp/ql/src/Best Practices/Hiding/LocalVariableHidesGlobalVariable.ql

@@ -2,7 +2,7 @@
  * @name Local variable hides global variable
  * @description A local variable or parameter that hides a global variable of the same name. This may be confusing. Consider renaming one of the variables.
  * @kind problem
- * @problem.severity warning
+ * @problem.severity recommendation
  * @precision very-high
  * @id cpp/local-variable-hides-global-variable
  * @tags maintainability

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

@@ -3,7 +3,7 @@
  */
 
 private import cpp
-private import semmle.code.cpp.models.implementations.Pure
+private import semmle.code.cpp.models.interfaces.SideEffect
 
 /**
  * A `Function` that is considered a "safe" external API from a security perspective.
@@ -13,9 +13,12 @@ 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
+    // If a function does not write to any of its arguments, we consider it safe to
+    // pass untrusted data to it. This means that string functions such as `strcmp`
+    // and `strlen`, as well as memory functions such as `memcmp`, are considered safe.
+    exists(SideEffectFunction model | model = this |
+      model.hasOnlySpecificWriteSideEffects() and
+      not model.hasSpecificWriteSideEffect(_, _, _)
+    )
   }
 }

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

@@ -3,7 +3,7 @@
  */
 
 private import cpp
-private import semmle.code.cpp.models.implementations.Pure
+private import semmle.code.cpp.models.interfaces.SideEffect
 
 /**
  * A `Function` that is considered a "safe" external API from a security perspective.
@@ -13,9 +13,12 @@ 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
+    // If a function does not write to any of its arguments, we consider it safe to
+    // pass untrusted data to it. This means that string functions such as `strcmp`
+    // and `strlen`, as well as memory functions such as `memcmp`, are considered safe.
+    exists(SideEffectFunction model | model = this |
+      model.hasOnlySpecificWriteSideEffects() and
+      not model.hasSpecificWriteSideEffect(_, _, _)
+    )
   }
 }

cpp/ql/src/jsf/4.07 Header Files/AV Rule 35.ql

@@ -4,7 +4,7 @@
  *              the file from being included twice). This prevents errors and
  *              inefficiencies caused by repeated inclusion.
  * @kind problem
- * @problem.severity warning
+ * @problem.severity recommendation
  * @precision high
  * @id cpp/missing-header-guard
  * @tags efficiency

cpp/ql/src/semmle/code/cpp/Class.qll

@@ -977,7 +977,12 @@ class ClassTemplateInstantiation extends Class {
  * specialization - see `FullClassTemplateSpecialization` and
  * `PartialClassTemplateSpecialization`).
  */
-abstract class ClassTemplateSpecialization extends Class {
+class ClassTemplateSpecialization extends Class {
+  ClassTemplateSpecialization() {
+    isFullClassTemplateSpecialization(this) or
+    isPartialClassTemplateSpecialization(this)
+  }
+
   /**
    * Gets the primary template for the specialization, for example on
    * `S<T,int>`, the result is `S<T,U>`.
@@ -997,6 +1002,16 @@ abstract class ClassTemplateSpecialization extends Class {
   override string getAPrimaryQlClass() { result = "ClassTemplateSpecialization" }
 }
 
+private predicate isFullClassTemplateSpecialization(Class c) {
+  // This class has template arguments, but none of them involves a template parameter.
+  exists(c.getATemplateArgument()) and
+  not exists(Type ta | ta = c.getATemplateArgument() and ta.involvesTemplateParameter()) and
+  // This class does not have any instantiations.
+  not exists(c.(TemplateClass).getAnInstantiation()) and
+  // This class is not an instantiation of a class template.
+  not c instanceof ClassTemplateInstantiation
+}
+
 /**
  * A full specialization of a class template.  For example `MyTemplateClass<int>`
  * in the following code is a `FullClassTemplateSpecialization`:
@@ -1013,19 +1028,31 @@ abstract class ClassTemplateSpecialization extends Class {
  * ```
  */
 class FullClassTemplateSpecialization extends ClassTemplateSpecialization {
-  FullClassTemplateSpecialization() {
-    // This class has template arguments, but none of them involves a template parameter.
-    exists(getATemplateArgument()) and
-    not exists(Type ta | ta = getATemplateArgument() and ta.involvesTemplateParameter()) and
-    // This class does not have any instantiations.
-    not exists(this.(TemplateClass).getAnInstantiation()) and
-    // This class is not an instantiation of a class template.
-    not this instanceof ClassTemplateInstantiation
-  }
+  FullClassTemplateSpecialization() { isFullClassTemplateSpecialization(this) }
 
   override string getAPrimaryQlClass() { result = "FullClassTemplateSpecialization" }
 }
 
+private predicate isPartialClassTemplateSpecialization(Class c) {
+  /*
+   * (a) At least one of this class's template arguments involves a
+   *     template parameter in some respect, for example T, T*, etc.
+   *
+   * (b) It is not the case that the n template arguments of this class
+   *     are a set of n distinct template parameters.
+   *
+   * template <typename T,U> class X {};      // class template
+   * template <typename T> class X<T,T> {};   // partial class template specialization
+   * template <typename T> class X<T,int> {}; // partial class template specialization
+   * template <typename T> class Y {};        // class template
+   * template <typename T> class Y<T*> {};    // partial class template specialization
+   */
+
+  exists(Type ta | ta = c.getATemplateArgument() and ta.involvesTemplateParameter()) and
+  count(TemplateParameter tp | tp = c.getATemplateArgument()) !=
+    count(int i | exists(c.getTemplateArgument(i)))
+}
+
 /**
  * A partial specialization of a class template.  For example `MyTemplateClass<int, T>`
  * in the following code is a `PartialClassTemplateSpecialization`:
@@ -1042,25 +1069,7 @@ class FullClassTemplateSpecialization extends ClassTemplateSpecialization {
  * ```
  */
 class PartialClassTemplateSpecialization extends ClassTemplateSpecialization {
-  PartialClassTemplateSpecialization() {
-    /*
-     * (a) At least one of this class's template arguments involves a
-     *     template parameter in some respect, for example T, T*, etc.
-     *
-     * (b) It is not the case that the n template arguments of this class
-     *     are a set of n distinct template parameters.
-     *
-     * template <typename T,U> class X {};      // class template
-     * template <typename T> class X<T,T> {};   // partial class template specialization
-     * template <typename T> class X<T,int> {}; // partial class template specialization
-     * template <typename T> class Y {};        // class template
-     * template <typename T> class Y<T*> {};    // partial class template specialization
-     */
-
-    exists(Type ta | ta = getATemplateArgument() and ta.involvesTemplateParameter()) and
-    count(TemplateParameter tp | tp = getATemplateArgument()) !=
-      count(int i | exists(getTemplateArgument(i)))
-  }
+  PartialClassTemplateSpecialization() { isPartialClassTemplateSpecialization(this) }
 
   override string getAPrimaryQlClass() { result = "PartialClassTemplateSpecialization" }
 }

cpp/ql/src/semmle/code/cpp/MemberFunction.qll

@@ -205,12 +205,21 @@ class Constructor extends MemberFunction {
 /**
  * A function that defines an implicit conversion.
  */
-abstract class ImplicitConversionFunction extends MemberFunction {
+class ImplicitConversionFunction extends MemberFunction {
+  ImplicitConversionFunction() {
+    // ConversionOperator
+    functions(underlyingElement(this), _, 4)
+    or
+    // ConversionConstructor (deprecated)
+    strictcount(Parameter p | p = getAParameter() and not p.hasInitializer()) = 1 and
+    not hasSpecifier("explicit")
+  }
+
   /** Gets the type this `ImplicitConversionFunction` takes as input. */
-  abstract Type getSourceType();
+  Type getSourceType() { none() } // overridden in subclasses
 
   /** Gets the type this `ImplicitConversionFunction` converts to. */
-  abstract Type getDestType();
+  Type getDestType() { none() } // overridden in subclasses
 }
 
 /**

cpp/ql/src/semmle/code/cpp/Print.qll

@@ -60,7 +60,7 @@ private string getTemplateArgumentString(Declaration d, int i) {
 /**
  * A `Declaration` extended to add methods for generating strings useful only for dumps and debugging.
  */
-abstract private class DumpDeclaration extends Declaration {
+private class DumpDeclaration extends Declaration {
   DumpDeclaration() { shouldPrintDeclaration(this) }
 
   /**

cpp/ql/src/semmle/code/cpp/Type.qll

@@ -577,7 +577,9 @@ class BoolType extends IntegralType {
  * unsigned char e, f;
  * ```
  */
-abstract class CharType extends IntegralType { }
+class CharType extends IntegralType {
+  CharType() { builtintypes(underlyingElement(this), _, [5, 6, 7], _, _, _) }
+}
 
 /**
  * The C/C++ `char` type (which is distinct from `signed char` and

cpp/ql/src/semmle/code/cpp/XML.qll

@@ -4,8 +4,11 @@
 
 import semmle.files.FileSystem
 
+private class TXMLLocatable =
+  @xmldtd or @xmlelement or @xmlattribute or @xmlnamespace or @xmlcomment or @xmlcharacters;
+
 /** An XML element that has a location. */
-abstract class XMLLocatable extends @xmllocatable {
+class XMLLocatable extends @xmllocatable, TXMLLocatable {
   /** Gets the source location for this element. */
   Location getLocation() { xmllocations(this, result) }
 
@@ -33,7 +36,7 @@ abstract class XMLLocatable extends @xmllocatable {
   }
 
   /** Gets a textual representation of this element. */
-  abstract string toString();
+  string toString() { none() } // overridden in subclasses
 }
 
 /**
@@ -51,7 +54,7 @@ class XMLParent extends @xmlparent {
    * Gets a printable representation of this XML parent.
    * (Intended to be overridden in subclasses.)
    */
-  abstract string getName();
+  string getName() { none() } // overridden in subclasses
 
   /** Gets the file to which this XML parent belongs. */
   XMLFile getFile() { result = this or xmlElements(this, _, _, _, result) }