Merge branch 'master' into rdmarsh/cpp/default-taint-tracking-sources

csharp/ql/src/Security Features/CWE-016/ASPNetPagesValidateRequest.qhelp

@@ -0,0 +1,51 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+
+	<overview>
+		<p>
+			Request validation is a feature in ASP.NET that protects web applications against
+			potentially malicious content in requests, specifically against
+			cross-site scripting attacks (XSS).
+		</p>
+
+	</overview>
+	<recommendation>
+
+		<p>
+			Enable the directive <code>validateRequest</code> in your <code>web.config</code> file:
+
+			<code>
+				&lt;pages validateRequest="true" /&gt;
+			</code>
+		</p>
+
+	</recommendation>
+	<example>
+
+		<p>
+			The following example shows the <code>validateRequest</code> flag set to <code>false</code>
+			in a <code>Web.config</code> file for ASP.NET. This will disable validation, and leave
+			the web application vulnerable against common XSS attacks:
+		</p>
+
+		<sample src="ASPNetPagesValidateRequestBad.config" />
+
+		<p>
+			If <code>validateRequest</code> is set to <code>true</code>, validation is enabled:
+		</p>
+
+		<sample src="ASPNetPagesValidateRequestGood.config" />
+
+	</example>
+	<references>
+
+		<li>
+			MSDN:
+			<a
+				href="https://docs.microsoft.com/en-us/previous-versions/aspnet/hh882339(v=vs.110)?redirectedfrom=MSDN">
+				Request Validation in ASP.NET
+			</a>.
+		</li>
+
+	</references>
+</qhelp>

csharp/ql/src/Security Features/CWE-016/ASPNetPagesValidateRequest.ql

@@ -0,0 +1,19 @@
+/**
+ * @name Page request validation is disabled
+ * @description ASP.NET pages should not disable the built-in request validation.
+ * @kind problem
+ * @problem.severity warning
+ * @id cs/web/request-validation-disabled
+ * @tags security
+ *       frameworks/asp.net
+ *       external/cwe/cwe-16
+ */
+
+import csharp
+import semmle.code.asp.WebConfig
+
+from SystemWebXMLElement web, XMLAttribute requestvalidateAttribute
+where
+  requestvalidateAttribute = web.getAChild("pages").getAttribute("validateRequest") and
+  requestvalidateAttribute.getValue().toLowerCase() = "false"
+select requestvalidateAttribute, "The 'validateRequest' attribute is set to 'false'."

csharp/ql/src/Security Features/CWE-016/ASPNetPagesValidateRequestBad.config

@@ -0,0 +1,5 @@
+<configuration>
+  <system.web>
+    <pages validateRequest="false" />
+  </system.web>
+</configuration>

csharp/ql/src/Security Features/CWE-016/ASPNetPagesValidateRequestGood.config

@@ -0,0 +1,5 @@
+<configuration>
+  <system.web>
+    <pages validateRequest="true" />
+  </system.web>
+</configuration>

csharp/ql/src/semmle/code/csharp/Caching.qll

@@ -89,7 +89,9 @@ module Stages {
 
     cached
     private predicate forceCachingInSameStageRev() {
-      exists(CompoundTypeKind k)
+      exists(Gvn::CompoundTypeKind k)
+      or
+      exists(any(Gvn::GvnType t).toString())
       or
       exists(Unification::UnconstrainedTypeParameter utp)
       or

csharp/ql/src/semmle/code/csharp/Conversion.qll

@@ -24,20 +24,25 @@ private module Cached {
    *
    * 6.1: Implicit type conversions.
    *
-   * The following conversions are classified as implicit conversions:
+   * The following conversions are included:
    *
    * - Identity conversions
    * - Implicit numeric conversions
    * - Implicit nullable conversions
    * - Implicit reference conversions
    * - Boxing conversions
-   * - User-defined implicit conversions
    */
   cached
-  predicate implicitConversion(Type fromType, Type toType) {
-    implicitConversionNonNull(fromType, toType)
+  predicate implicitConversionRestricted(Type fromType, Type toType) {
+    convIdentity(fromType, toType)
     or
-    defaultNullConversion(fromType, toType)
+    convNumeric(fromType, toType)
+    or
+    convNullableType(fromType, toType)
+    or
+    convRefTypeNonNull(fromType, toType)
+    or
+    convBoxing(fromType, toType)
   }
 
   /**
@@ -58,21 +63,36 @@ private module Cached {
 import Cached
 
 private predicate implicitConversionNonNull(Type fromType, Type toType) {
-  convIdentity(fromType, toType)
-  or
-  convNumeric(fromType, toType)
-  or
-  convNullableType(fromType, toType)
-  or
-  convRefTypeNonNull(fromType, toType)
-  or
-  convBoxing(fromType, toType)
+  implicitConversionRestricted(fromType, toType)
   or
   convConversionOperator(fromType, toType)
   or
   fromType instanceof DynamicType // 6.1.8
 }
 
+/**
+ * INTERNAL: Do not use.
+ *
+ * Holds if there exists an implicit conversion from `fromType` to `toType`.
+ *
+ * 6.1: Implicit type conversions.
+ *
+ * The following conversions are classified as implicit conversions:
+ *
+ * - Identity conversions
+ * - Implicit numeric conversions
+ * - Implicit nullable conversions
+ * - Implicit reference conversions
+ * - Boxing conversions
+ * - User-defined implicit conversions
+ */
+pragma[nomagic]
+predicate implicitConversion(Type fromType, Type toType) {
+  implicitConversionNonNull(fromType, toType)
+  or
+  defaultNullConversion(fromType, toType)
+}
+
 /**
  * A generic type. This includes both constructed generic types and unbound
  * generic types (which correspond to constructed generic types where the

csharp/ql/src/semmle/code/csharp/Implements.qll

@@ -130,7 +130,7 @@ private DeclarationWithAccessors getACompatibleInterfaceAccessorCandidate(Declar
   d.isPublic()
 }
 
-pragma[noinline]
+pragma[nomagic]
 private predicate getACompatibleInterfaceAccessorAux(
   DeclarationWithAccessors d, ValueOrRefType t, string name
 ) {
@@ -242,7 +242,7 @@ private Type getArgumentOrReturnType(Method m, int i) {
  * type parameters (at the same index).
  */
 private module Gvn {
-  private import semmle.code.csharp.Unification
+  private import semmle.code.csharp.Unification::Gvn as Unification
 
   private class MethodTypeParameter extends TypeParameter {
     MethodTypeParameter() { this = any(UnboundGenericMethod ugm).getATypeParameter() }
@@ -251,27 +251,26 @@ private module Gvn {
   private class LeafType extends Type {
     LeafType() {
       not exists(this.getAChild()) and
-      not this instanceof MethodTypeParameter
+      not this instanceof MethodTypeParameter and
+      not this instanceof DynamicType
     }
   }
 
-  private predicate id(LeafType t, int i) = equivalenceRelation(convIdentity/2)(t, i)
-
   private newtype TGvnType =
-    TLeafGvnType(int i) { id(_, i) } or
+    TLeafGvnType(LeafType t) or
     TMethodTypeParameterGvnType(int i) { i = any(MethodTypeParameter p).getIndex() } or
     TConstructedGvnType(ConstructedGvnTypeList l)
 
   private newtype TConstructedGvnTypeList =
-    TConstructedGvnTypeNil(CompoundTypeKind k) or
+    TConstructedGvnTypeNil(Unification::CompoundTypeKind k) or
     TConstructedGvnTypeCons(GvnType head, ConstructedGvnTypeList tail) {
       gvnConstructedCons(_, _, _, head, tail)
     }
 
-  private ConstructedGvnTypeList gvnConstructed(Type t, CompoundTypeKind k, int i) {
+  private ConstructedGvnTypeList gvnConstructed(Type t, Unification::CompoundTypeKind k, int i) {
     result = TConstructedGvnTypeNil(k) and
     i = -1 and
-    k = getTypeKind(t)
+    k = Unification::getTypeKind(t)
     or
     exists(GvnType head, ConstructedGvnTypeList tail | gvnConstructedCons(t, k, i, head, tail) |
       result = TConstructedGvnTypeCons(head, tail)
@@ -283,7 +282,7 @@ private module Gvn {
 
   pragma[noinline]
   private predicate gvnConstructedCons(
-    Type t, CompoundTypeKind k, int i, GvnType head, ConstructedGvnTypeList tail
+    Type t, Unification::CompoundTypeKind k, int i, GvnType head, ConstructedGvnTypeList tail
   ) {
     tail = gvnConstructed(t, k, i - 1) and
     head = gvnTypeChild(t, i)
@@ -292,11 +291,14 @@ private module Gvn {
   /** Gets the global value number for a given type. */
   pragma[nomagic]
   GvnType getGlobalValueNumber(Type t) {
-    result = TLeafGvnType(any(int i | id(t, i)))
+    result = TLeafGvnType(t)
+    or
+    t instanceof DynamicType and
+    result = TLeafGvnType(any(ObjectType ot))
     or
     result = TMethodTypeParameterGvnType(t.(MethodTypeParameter).getIndex())
     or
-    exists(ConstructedGvnTypeList l, CompoundTypeKind k, int i |
+    exists(ConstructedGvnTypeList l, Unification::CompoundTypeKind k, int i |
       l = gvnConstructed(t, k, i) and
       i = k.getNumberOfTypeParameters() - 1 and
       result = TConstructedGvnType(l)
@@ -306,7 +308,7 @@ private module Gvn {
   /** A global value number for a type. */
   class GvnType extends TGvnType {
     string toString() {
-      exists(int i | this = TLeafGvnType(i) | result = i.toString())
+      exists(LeafType t | this = TLeafGvnType(t) | result = t.toString())
       or
       exists(int i | this = TMethodTypeParameterGvnType(i) | result = "M!" + i)
       or
@@ -338,12 +340,12 @@ private module Gvn {
 
     language[monotonicAggregates]
     string toString() {
-      exists(CompoundTypeKind k, string args |
+      exists(Unification::CompoundTypeKind k, string args |
         this = gvnConstructed(_, k, _) and
         args = concat(int i |
             i in [0 .. k.getNumberOfTypeParameters() - 1]
           |
-            this.getArg(i).toString(), ", " order by i
+            this.getArg(i).toString(), "," order by i
           ) and
         result = k.toString(args)
       )

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

@@ -827,16 +827,13 @@ class ArrayType extends DotNet::ArrayType, RefType, @array_type {
     getRank() = that.getRank()
   }
 
-  private string getRankString(int i) {
-    i in [0 .. getRank() - 1] and
-    if i = getRank() - 1 then result = "" else result = "," + getRankString(i + 1)
-  }
-
   /**
    * INTERNAL: Do not use.
    * Gets a string representing the array suffix, for example `[,,,]`.
    */
-  string getArraySuffix() { result = "[" + getRankString(0) + "]" }
+  string getArraySuffix() {
+    result = "[" + concat(int i | i in [0 .. this.getRank() - 2] | ",") + "]"
+  }
 
   private string getDimensionString(Type elementType) {
     exists(Type et, string res |

csharp/ql/src/semmle/code/csharp/Unification.qll

@@ -2,28 +2,24 @@ import csharp
 private import Conversion
 private import Caching
 
-pragma[noinline]
-private Type getAProperSubType(Type t) {
-  not result instanceof DynamicType and
-  not result instanceof NullType and
-  result.isImplicitlyConvertibleTo(t)
-}
-
 /**
+ * INTERNAL: Do not use.
+ *
  * Provides an implementation of Global Value Numbering for types (see
  * https://en.wikipedia.org/wiki/Global_value_numbering), where types are considered
  * equal modulo identity conversions and type parameters.
  */
-private module Gvn {
+module Gvn {
   private class LeafType extends Type {
     LeafType() {
       not exists(this.getAChild()) and
-      not this instanceof TypeParameter
+      not this instanceof TypeParameter and
+      not this instanceof DynamicType
     }
   }
 
   /** A type kind for a compound type. */
-  class CompoundTypeKindImpl extends TCompoundTypeKind {
+  class CompoundTypeKind extends TCompoundTypeKind {
     /** Gets the number of type parameters for this kind. */
     int getNumberOfTypeParameters() {
       this = TPointerTypeKind() and result = 1
@@ -44,8 +40,8 @@ private module Gvn {
       or
       this = TNullableTypeKind() and result = args + "?"
       or
-      exists(int dim, int rnk | this = TArrayTypeKind(dim, rnk) |
-        result = args + "[" + dim + ", " + rnk + "]"
+      exists(int rnk | this = TArrayTypeKind(_, rnk) |
+        result = args + "[" + concat(int i | i in [0 .. rnk - 2] | ",") + "]"
       )
       or
       exists(UnboundGenericType ugt | this = TConstructedType(ugt) |
@@ -61,7 +57,7 @@ private module Gvn {
   }
 
   /** Gets the type kind for type `t`, if any. */
-  CompoundTypeKind getTypeKindImpl(Type t) {
+  CompoundTypeKind getTypeKind(Type t) {
     result = TPointerTypeKind() and t instanceof PointerType
     or
     result = TNullableTypeKind() and t instanceof NullableType
@@ -86,11 +82,13 @@ private module Gvn {
     CompoundTypeKind getKind() { none() }
 
     /** Gets a textual representation of this GVN. */
+    cached
     string toString() {
-      exists(int i | this = TLeafGvnType(i) | result = i.toString())
+      Stages::UnificationStage::forceCachingInSameStage() and
+      exists(LeafType t | this = TLeafGvnType(t) | result = t.toString())
       or
       this instanceof TTypeParameterGvnType and
-      result = "<type parameter>"
+      result = "T"
       or
       exists(ConstructedGvnTypeList l | this = TConstructedGvnType(l) | result = l.toString())
     }
@@ -99,6 +97,8 @@ private module Gvn {
     Location getLocation() { result instanceof EmptyLocation }
   }
 
+  class TypeParameterGvnType extends GvnType, TTypeParameterGvnType { }
+
   class ConstructedGvnType extends GvnType, TConstructedGvnType {
     private ConstructedGvnTypeList l;
 
@@ -157,7 +157,7 @@ private module Gvn {
         args = concat(int i |
             i in [0 .. k.getNumberOfTypeParameters() - 1]
           |
-            this.getArg(i).toString(), ", " order by i
+            this.getArg(i).toString(), "," order by i
           ) and
         result = k.toString(args)
       )
@@ -356,8 +356,6 @@ private module Gvn {
     not result instanceof ConstructedGvnType
   }
 
-  private predicate id(LeafType t, int i) = equivalenceRelation(convIdentity/2)(t, i)
-
   cached
   private module Cached {
     cached
@@ -367,11 +365,11 @@ private module Gvn {
       TArrayTypeKind(int dim, int rnk) {
         exists(ArrayType at | dim = at.getDimension() and rnk = at.getRank())
       } or
-      TConstructedType(UnboundGenericType ugt)
+      TConstructedType(UnboundGenericType ugt) { exists(ugt.getATypeParameter()) }
 
     cached
     newtype TGvnType =
-      TLeafGvnType(int i) { id(_, i) } or
+      TLeafGvnType(LeafType t) or
       TTypeParameterGvnType() or
       TConstructedGvnType(ConstructedGvnTypeList l)
 
@@ -385,7 +383,10 @@ private module Gvn {
     /** Gets the GVN for type `t`. */
     cached
     GvnType getGlobalValueNumber(Type t) {
-      result = TLeafGvnType(any(int i | id(t, i)))
+      result = TLeafGvnType(t)
+      or
+      t instanceof DynamicType and
+      result = TLeafGvnType(any(ObjectType ot))
       or
       t instanceof TypeParameter and
       result = TTypeParameterGvnType()
@@ -430,10 +431,6 @@ private module Gvn {
   import Cached
 }
 
-class CompoundTypeKind = Gvn::CompoundTypeKindImpl;
-
-predicate getTypeKind = Gvn::getTypeKindImpl/1;
-
 /** Provides definitions related to type unification. */
 module Unification {
   /** A type parameter that is compatible with any type. */
@@ -557,19 +554,19 @@ module Unification {
 
     cached
     predicate typeConstraintUnifiable(TTypeConstraint ttc, Type t) {
-      exists(Type t0 | ttc = TTypeConstraint(t0) | t = getAProperSubType(t0))
+      exists(Type t0 | ttc = TTypeConstraint(t0) | implicitConversionRestricted(t, t0))
       or
       exists(Type t0, Type t1 | ttc = TTypeConstraint(t0) and unifiable(t0, t1) |
-        t = getAProperSubType(t1)
+        implicitConversionRestricted(t, t1)
       )
     }
 
     cached
     predicate typeConstraintSubsumes(TTypeConstraint ttc, Type t) {
-      exists(Type t0 | ttc = TTypeConstraint(t0) | t = getAProperSubType(t0))
+      exists(Type t0 | ttc = TTypeConstraint(t0) | implicitConversionRestricted(t, t0))
       or
       exists(Type t0, Type t1 | ttc = TTypeConstraint(t0) and subsumes(t0, t1) |
-        t = getAProperSubType(t1)
+        implicitConversionRestricted(t, t1)
       )
     }
   }

csharp/ql/src/semmle/code/csharp/commons/Assertions.qll

@@ -1,6 +1,7 @@
 /** Provides classes for assertions. */
 
 private import semmle.code.csharp.frameworks.system.Diagnostics
+private import semmle.code.csharp.frameworks.system.diagnostics.Contracts
 private import semmle.code.csharp.frameworks.test.VisualStudio
 private import semmle.code.csharp.frameworks.System
 private import ControlFlow
@@ -169,6 +170,29 @@ class SystemDiagnosticsDebugAssertTrueMethod extends AssertTrueMethod {
   }
 }
 
+/**
+ * A `System.Diagnostics.Contracts.Contract` assertion method.
+ */
+class SystemDiagnosticsContractAssertTrueMethod extends AssertTrueMethod {
+  SystemDiagnosticsContractAssertTrueMethod() {
+    exists(SystemDiagnosticsContractsContractClass c |
+      this = c.getAnAssertMethod()
+      or
+      this = c.getAnAssumeMethod()
+      or
+      this = c.getARequiresMethod()
+    )
+  }
+
+  override int getAssertionIndex() { result = 0 }
+
+  override Class getExceptionClass() {
+    // A failing assertion generates a message box, see
+    // https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.contracts.contract.assert
+    none()
+  }
+}
+
 /** A Visual Studio assertion method. */
 class VSTestAssertTrueMethod extends AssertTrueMethod {
   VSTestAssertTrueMethod() { this = any(VSTestAssertClass c).getIsTrueMethod() }

csharp/ql/src/semmle/code/csharp/dataflow/SSA.qll

@@ -1765,7 +1765,7 @@ module Ssa {
      *
      * The write is live because of the implicit entry definition `def`, which can be
      * reached using one or more calls (as indicated by `additionalCalls`), starting
-     * from call `c`. That is, data can flow from the write at index `i` into the the
+     * from call `c`. That is, data can flow from the write at index `i` into the
      * callable containing `def`.
      *
      * Example:
@@ -2329,7 +2329,7 @@ module Ssa {
      * ```
      *
      * If this definition is the update of `i` on line 5, then the value may be read inside
-     * `M2` via the the call on line 6.
+     * `M2` via the call on line 6.
      */
     predicate isCapturedVariableDefinitionFlowIn(
       ImplicitEntryDefinition def, ControlFlow::Nodes::ElementNode c, boolean additionalCalls
@@ -2356,7 +2356,7 @@ module Ssa {
      * ```
      *
      * If this definition is the update of `i` on line 4, then the value may be read outside
-     * of `M2` via the the call on line 5.
+     * of `M2` via the call on line 5.
      */
     predicate isCapturedVariableDefinitionFlowOut(
       ImplicitCallDefinition cdef, boolean additionalCalls

csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll

@@ -7,7 +7,9 @@ private import DataFlowImplCommon::Public
 private import ControlFlowReachability
 private import DelegateDataFlow
 private import semmle.code.csharp.Caching
+private import semmle.code.csharp.Conversion
 private import semmle.code.csharp.ExprOrStmtParent
+private import semmle.code.csharp.Unification
 private import semmle.code.csharp.controlflow.Guards
 private import semmle.code.csharp.dataflow.LibraryTypeDataFlow
 private import semmle.code.csharp.dispatch.Dispatch
@@ -297,7 +299,7 @@ private class Argument extends Expr {
 private predicate instanceFieldLikeAssign(Expr e, FieldLike f, Expr src, Expr q) {
   exists(FieldLikeAccess fa, AssignableDefinition def |
     def.getTargetAccess() = fa and
-    f = fa.getTarget() and
+    f = fa.getTarget().getSourceDeclaration() and
     not f.isStatic() and
     src = def.getSource() and
     q = fa.getQualifier() and
@@ -312,12 +314,35 @@ private predicate instanceFieldLikeAssign(Expr e, FieldLike f, Expr src, Expr q)
 private predicate instanceFieldLikeInit(ObjectCreation oc, FieldLike f, Expr src) {
   exists(MemberInitializer mi |
     mi = oc.getInitializer().(ObjectInitializer).getAMemberInitializer() and
-    f = mi.getInitializedMember() and
+    f = mi.getInitializedMember().getSourceDeclaration() and
     not f.isStatic() and
     src = mi.getRValue()
   )
 }
 
+private Type getCSharpType(DotNet::Type t) {
+  result = t
+  or
+  result.matchesHandle(t)
+}
+
+pragma[noinline]
+private TypeParameter getATypeParameterSubType(DataFlowType t) {
+  not t instanceof Gvn::TypeParameterGvnType and
+  exists(Type t0 | t = Gvn::getGlobalValueNumber(t0) | implicitConversionRestricted(result, t0))
+}
+
+pragma[noinline]
+private DataFlowType getANonTypeParameterSubType(DataFlowType t) {
+  not t instanceof Gvn::TypeParameterGvnType and
+  not result instanceof Gvn::TypeParameterGvnType and
+  exists(Type t1, Type t2 |
+    implicitConversionRestricted(t1, t2) and
+    result = Gvn::getGlobalValueNumber(t1) and
+    t = Gvn::getGlobalValueNumber(t2)
+  )
+}
+
 /** A collection of cached types and predicates to be evaluated in the same stage. */
 cached
 private module Cached {
@@ -414,7 +439,8 @@ private module Cached {
   }
 
   cached
-  newtype TContent = TFieldLikeContent(FieldLike f) { not f.isStatic() }
+  newtype TContent =
+    TFieldLikeContent(FieldLike f) { not f.isStatic() and f.getSourceDeclaration() = f }
 
   /**
    * Holds if data can flow from `node1` to `node2` via an assignment to
@@ -442,7 +468,11 @@ private module Cached {
   predicate readStepImpl(Node node1, Content c, Node node2) {
     exists(ReadStepConfiguration x |
       x.hasNodePath(node1, node2) and
-      c.(FieldLikeContent).getField() = node2.asExpr().(FieldLikeRead).getTarget()
+      c.(FieldLikeContent).getField() = node2
+            .asExpr()
+            .(FieldLikeRead)
+            .getTarget()
+            .getSourceDeclaration()
     )
   }
 
@@ -461,6 +491,41 @@ private module Cached {
       guard.controlsBlock(n.getControlFlowNode().getBasicBlock(), bs)
     )
   }
+
+  /**
+   * Gets a representative type for `t` for the purpose of pruning possible flow.
+   */
+  cached
+  DataFlowType getErasedRepr(DotNet::Type t) {
+    exists(Type t0 | result = Gvn::getGlobalValueNumber(t0) |
+      t0 = getCSharpType(t)
+      or
+      not exists(getCSharpType(t)) and
+      t0 instanceof ObjectType
+    )
+  }
+
+  /**
+   * Holds if GVNs `t1` and `t2` may have a common sub type. Neither `t1` nor
+   * `t2` are allowed to be type parameters.
+   */
+  cached
+  predicate commonSubType(DataFlowType t1, DataFlowType t2) {
+    not t1 instanceof Gvn::TypeParameterGvnType and
+    t1 = t2
+    or
+    getATypeParameterSubType(t1) = getATypeParameterSubType(t2)
+    or
+    getANonTypeParameterSubType(t1) = getANonTypeParameterSubType(t2)
+  }
+
+  cached
+  predicate commonSubTypeUnifiableLeft(DataFlowType t1, DataFlowType t2) {
+    exists(DataFlowType t |
+      Gvn::unifiable(t1, t) and
+      commonSubType(t, t2)
+    )
+  }
 }
 
 import Cached
@@ -571,7 +636,11 @@ private module ParameterNodes {
       result = this.getUnderlyingNode().getEnclosingCallable()
     }
 
-    override Type getType() { result = this.getUnderlyingNode().getType() }
+    override Type getType() {
+      // Taint tracking steps are allowed to change the type of the tracked object,
+      // so `result = this.getUnderlyingNode().getType()` is too restrictive
+      result instanceof ObjectType
+    }
 
     override Location getLocation() { result = this.getUnderlyingNode().getLocation() }
 
@@ -914,7 +983,9 @@ private module ReturnNodes {
       result = this.getUnderlyingNode().getEnclosingCallable()
     }
 
-    override Type getType() { result = this.getUnderlyingNode().getType() }
+    override Type getType() {
+      result = this.getUnderlyingNode().getEnclosingCallable().getReturnType()
+    }
 
     override Location getLocation() { result = this.getUnderlyingNode().getLocation() }
 
@@ -1105,7 +1176,11 @@ private module OutNodes {
 
     override Callable getEnclosingCallable() { result = cfn.getEnclosingCallable() }
 
-    override Type getType() { result = cfn.getElement().(Expr).getType() }
+    override Type getType() {
+      exists(ImplicitDelegateDataFlowCall c | c.getNode() = this |
+        result = c.getDelegateReturnType()
+      )
+    }
 
     override Location getLocation() { result = cfn.getLocation() }
 
@@ -1243,10 +1318,10 @@ class Content extends TContent {
   abstract Location getLocation();
 
   /** Gets the type of the object containing this content. */
-  abstract Type getContainerType();
+  abstract DataFlowType getContainerType();
 
   /** Gets the type of this content. */
-  abstract Type getType();
+  abstract DataFlowType getType();
 }
 
 private class FieldLikeContent extends Content, TFieldLikeContent {
@@ -1260,9 +1335,11 @@ private class FieldLikeContent extends Content, TFieldLikeContent {
 
   override Location getLocation() { result = f.getLocation() }
 
-  override Type getContainerType() { result = f.getDeclaringType() }
+  override DataFlowType getContainerType() {
+    result = Gvn::getGlobalValueNumber(f.getDeclaringType())
+  }
 
-  override Type getType() { result = f.getType() }
+  override DataFlowType getType() { result = Gvn::getGlobalValueNumber(f.getType()) }
 }
 
 private class StoreStepConfiguration extends ControlFlowReachabilityConfiguration {
@@ -1299,31 +1376,37 @@ private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration
 
 predicate readStep = readStepImpl/3;
 
-private predicate suppressUnusedType(DotNet::Type t) { any() }
-
-/**
- * Gets a representative type for `t` for the purpose of pruning possible flow.
- *
- * Type-based pruning is disabled for now, so this is a stub implementation.
- */
-bindingset[t]
-DataFlowType getErasedRepr(DotNet::Type t) {
-  // stub implementation
-  suppressUnusedType(t) and result instanceof ObjectType
-}
-
 /** Gets a string representation of a type returned by `getErasedRepr`. */
-string ppReprType(DotNet::Type t) { none() } // stub implementation
+string ppReprType(DataFlowType t) { result = t.toString() }
+
+private class DataFlowNullType extends DataFlowType {
+  DataFlowNullType() { this = Gvn::getGlobalValueNumber(any(NullType nt)) }
+
+  pragma[noinline]
+  predicate isConvertibleTo(DataFlowType t) {
+    defaultNullConversion(_, any(Type t0 | t = Gvn::getGlobalValueNumber(t0)))
+  }
+}
 
 /**
  * Holds if `t1` and `t2` are compatible, that is, whether data can flow from
  * a node of type `t1` to a node of type `t2`.
- *
- * Type-based pruning is disabled for now, so this is a stub implementation.
  */
-bindingset[t1, t2]
-predicate compatibleTypes(DotNet::Type t1, DotNet::Type t2) {
-  any() // stub implementation
+pragma[inline]
+predicate compatibleTypes(DataFlowType t1, DataFlowType t2) {
+  commonSubType(t1, t2)
+  or
+  commonSubTypeUnifiableLeft(t1, t2)
+  or
+  commonSubTypeUnifiableLeft(t2, t1)
+  or
+  t1.(DataFlowNullType).isConvertibleTo(t2)
+  or
+  t2.(DataFlowNullType).isConvertibleTo(t1)
+  or
+  t1 instanceof Gvn::TypeParameterGvnType
+  or
+  t2 instanceof Gvn::TypeParameterGvnType
 }
 
 /**
@@ -1370,13 +1453,24 @@ private module PostUpdateNodes {
 private import PostUpdateNodes
 
 /** A node that performs a type cast. */
-class CastNode extends ExprNode {
-  CastNode() { this.getExpr() instanceof CastExpr }
+class CastNode extends Node {
+  CastNode() {
+    this.asExpr() instanceof Cast
+    or
+    exists(Ssa::ExplicitDefinition def |
+      def = this.(SsaDefinitionNode).getDefinition() and
+      def.getADefinition() instanceof AssignableDefinitions::PatternDefinition
+    )
+    or
+    readStep(_, _, this)
+    or
+    storeStep(this, _, _)
+  }
 }
 
 class DataFlowExpr = DotNet::Expr;
 
-class DataFlowType = DotNet::Type;
+class DataFlowType = Gvn::GvnType;
 
 class DataFlowLocation = Location;
 

csharp/ql/src/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll

@@ -117,6 +117,7 @@ private class LocalTaintExprStepConfiguration extends ControlFlowReachabilityCon
       e2 = any(OperatorCall oc |
           oc.getTarget().(ConversionOperator).fromLibrary() and
           e1 = oc.getAnArgument() and
+          scope = e2 and
           isSuccessor = true
         )
     )

csharp/ql/src/semmle/code/csharp/frameworks/system/Diagnostics.qll

@@ -23,7 +23,7 @@ class SystemDiagnosticsDebugClass extends SystemDiagnosticsClass {
     this.isStatic()
   }
 
-  /** Gets and `Assert(bool, ...)` method. */
+  /** Gets an `Assert(bool, ...)` method. */
   Method getAssertMethod() {
     result.getDeclaringType() = this and
     result.hasName("Assert") and

csharp/ql/src/semmle/code/csharp/frameworks/system/diagnostics/Contracts.qll

@@ -0,0 +1,51 @@
+/** Provides definitions related to the namespace `System.Diagnostics.Contracts`. */
+
+import semmle.code.csharp.Type
+private import semmle.code.csharp.frameworks.system.Diagnostics
+
+/** The `System.Diagnostics.Contracts` namespace. */
+class SystemDiagnosticsContractsNamespace extends Namespace {
+  SystemDiagnosticsContractsNamespace() {
+    this.getParentNamespace() instanceof SystemDiagnosticsNamespace and
+    this.hasName("Contracts")
+  }
+}
+
+/** A class in the `System.Diagnostics.Contracts` namespace. */
+class SystemDiagnosticsContractsClass extends Class {
+  SystemDiagnosticsContractsClass() {
+    this.getNamespace() instanceof SystemDiagnosticsContractsNamespace
+  }
+}
+
+/** The `System.Diagnostics.Contracts.Contract` class. */
+class SystemDiagnosticsContractsContractClass extends SystemDiagnosticsContractsClass {
+  SystemDiagnosticsContractsContractClass() {
+    this.hasName("Contract") and
+    this.isStatic()
+  }
+
+  /** Gets an `Assert(bool, ...)` method. */
+  Method getAnAssertMethod() {
+    result.getDeclaringType() = this and
+    result.hasName("Assert") and
+    result.getParameter(0).getType() instanceof BoolType and
+    result.getReturnType() instanceof VoidType
+  }
+
+  /** Gets an `Assume(bool, ...)` method. */
+  Method getAnAssumeMethod() {
+    result.getDeclaringType() = this and
+    result.hasName("Assume") and
+    result.getParameter(0).getType() instanceof BoolType and
+    result.getReturnType() instanceof VoidType
+  }
+
+  /** Gets a `Requires(bool, ...)` method. */
+  Method getARequiresMethod() {
+    result.getDeclaringType() = this and
+    result.hasName("Requires") and
+    result.getParameter(0).getType() instanceof BoolType and
+    result.getReturnType() instanceof VoidType
+  }
+}

csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Operand.qll

@@ -11,13 +11,13 @@ cached
 private newtype TOperand =
   TRegisterOperand(Instruction useInstr, RegisterOperandTag tag, Instruction defInstr) {
     defInstr = Construction::getRegisterOperandDefinition(useInstr, tag) and
-    not isInCycle(useInstr)
+    not Construction::isInCycle(useInstr)
   } or
   TNonPhiMemoryOperand(
     Instruction useInstr, MemoryOperandTag tag, Instruction defInstr, Overlap overlap
   ) {
     defInstr = Construction::getMemoryOperandDefinition(useInstr, tag, overlap) and
-    not isInCycle(useInstr)
+    not Construction::isInCycle(useInstr)
   } or
   TPhiOperand(
     PhiInstruction useInstr, Instruction defInstr, IRBlock predecessorBlock, Overlap overlap
@@ -25,28 +25,6 @@ private newtype TOperand =
     defInstr = Construction::getPhiOperandDefinition(useInstr, predecessorBlock, overlap)
   }
 
-/** Gets a non-phi instruction that defines an operand of `instr`. */
-private Instruction getNonPhiOperandDef(Instruction instr) {
-  result = Construction::getRegisterOperandDefinition(instr, _)
-  or
-  result = Construction::getMemoryOperandDefinition(instr, _, _)
-}
-
-/**
- * Holds if `instr` is part of a cycle in the operand graph that doesn't go
- * through a phi instruction and therefore should be impossible.
- *
- * If such cycles are present, either due to a programming error in the IR
- * generation or due to a malformed database, it can cause infinite loops in
- * analyses that assume a cycle-free graph of non-phi operands. Therefore it's
- * better to remove these operands than to leave cycles in the operand graph.
- */
-pragma[noopt]
-private predicate isInCycle(Instruction instr) {
-  instr instanceof Instruction and
-  getNonPhiOperandDef+(instr) = instr
-}
-
 /**
  * A source operand of an `Instruction`. The operand represents a value consumed by the instruction.
  */

csharp/ql/src/semmle/code/csharp/ir/implementation/raw/internal/IRConstruction.qll

@@ -95,6 +95,29 @@ private module Cached {
           .getInstructionOperand(getInstructionTag(instruction), tag)
   }
 
+  /** Gets a non-phi instruction that defines an operand of `instr`. */
+  private Instruction getNonPhiOperandDef(Instruction instr) {
+    result = getRegisterOperandDefinition(instr, _)
+    or
+    result = getMemoryOperandDefinition(instr, _, _)
+  }
+
+  /**
+   * Holds if `instr` is part of a cycle in the operand graph that doesn't go
+   * through a phi instruction and therefore should be impossible.
+   *
+   * If such cycles are present, either due to a programming error in the IR
+   * generation or due to a malformed database, it can cause infinite loops in
+   * analyses that assume a cycle-free graph of non-phi operands. Therefore it's
+   * better to remove these operands than to leave cycles in the operand graph.
+   */
+  pragma[noopt]
+  cached
+  predicate isInCycle(Instruction instr) {
+    instr instanceof Instruction and
+    getNonPhiOperandDef+(instr) = instr
+  }
+
   cached
   CSharpType getInstructionOperandType(Instruction instruction, TypedOperandTag tag) {
     // For all `LoadInstruction`s, the operand type of the `LoadOperand` is the same as

csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Operand.qll

@@ -11,13 +11,13 @@ cached
 private newtype TOperand =
   TRegisterOperand(Instruction useInstr, RegisterOperandTag tag, Instruction defInstr) {
     defInstr = Construction::getRegisterOperandDefinition(useInstr, tag) and
-    not isInCycle(useInstr)
+    not Construction::isInCycle(useInstr)
   } or
   TNonPhiMemoryOperand(
     Instruction useInstr, MemoryOperandTag tag, Instruction defInstr, Overlap overlap
   ) {
     defInstr = Construction::getMemoryOperandDefinition(useInstr, tag, overlap) and
-    not isInCycle(useInstr)
+    not Construction::isInCycle(useInstr)
   } or
   TPhiOperand(
     PhiInstruction useInstr, Instruction defInstr, IRBlock predecessorBlock, Overlap overlap
@@ -25,28 +25,6 @@ private newtype TOperand =
     defInstr = Construction::getPhiOperandDefinition(useInstr, predecessorBlock, overlap)
   }
 
-/** Gets a non-phi instruction that defines an operand of `instr`. */
-private Instruction getNonPhiOperandDef(Instruction instr) {
-  result = Construction::getRegisterOperandDefinition(instr, _)
-  or
-  result = Construction::getMemoryOperandDefinition(instr, _, _)
-}
-
-/**
- * Holds if `instr` is part of a cycle in the operand graph that doesn't go
- * through a phi instruction and therefore should be impossible.
- *
- * If such cycles are present, either due to a programming error in the IR
- * generation or due to a malformed database, it can cause infinite loops in
- * analyses that assume a cycle-free graph of non-phi operands. Therefore it's
- * better to remove these operands than to leave cycles in the operand graph.
- */
-pragma[noopt]
-private predicate isInCycle(Instruction instr) {
-  instr instanceof Instruction and
-  getNonPhiOperandDef+(instr) = instr
-}
-
 /**
  * A source operand of an `Instruction`. The operand represents a value consumed by the instruction.
  */

csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll

@@ -133,6 +133,16 @@ private module Cached {
     overlap instanceof MustExactlyOverlap
   }
 
+  /**
+   * Holds if `instr` is part of a cycle in the operand graph that doesn't go
+   * through a phi instruction and therefore should be impossible.
+   *
+   * For performance reasons, this predicate is not implemented (never holds)
+   * for the SSA stages of the IR.
+   */
+  cached
+  predicate isInCycle(Instruction instr) { none() }
+
   cached
   Language::LanguageType getInstructionOperandType(Instruction instr, TypedOperandTag tag) {
     exists(OldInstruction oldInstruction, OldIR::TypedOperand oldOperand |

csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/reachability/ReachableBlockInternal.qll

@@ -1,2 +1,2 @@
-import semmle.code.cpp.ir.implementation.unaliased_ssa.IR as IR
-import semmle.code.cpp.ir.implementation.unaliased_ssa.constant.ConstantAnalysis as ConstantAnalysis
+import semmle.code.csharp.ir.implementation.unaliased_ssa.IR as IR
+import semmle.code.csharp.ir.implementation.unaliased_ssa.constant.ConstantAnalysis as ConstantAnalysis

csharp/ql/src/semmle/code/csharp/ir/internal/TIRVariable.qll

@@ -9,6 +9,8 @@ newtype TIRVariable =
     Construction::functionHasIR(callable) and
     var.getCallable() = callable
   } or
-  TIRTempVariable(Callable callable, Language::AST ast, TempVariableTag tag, Type type) {
+  TIRTempVariable(
+    Callable callable, Language::AST ast, TempVariableTag tag, Language::LanguageType type
+  ) {
     Construction::hasTempVariable(callable, ast, tag, type)
   }