Merge branch 'main' into shared-basic-block-library

csharp/ql/lib/CHANGELOG.md

@@ -1,3 +1,24 @@
+## 4.0.2
+
+### Minor Analysis Improvements
+
+* Added extractor support for extracting implicit `ToString` calls in binary `+` expressions and string interpolation expressions.
+* The Razor source generator invocation in `build-mode:none` extraction has been changed to use relative file paths instead of absolute ones.
+* C# 13: Added extractor support and call dispatch logic (data flow) for the (negative) type parameter constraint `allows ref struct`. Added extractor support for the type parameter constraint `notnull`.
+
+## 4.0.1
+
+### Minor Analysis Improvements
+
+* C# 13: Added QL library support for *collection* like type `params` parameters.
+* Added `remote` flow source models for properties of Blazor components annotated with any of the following attributes from `Microsoft.AspNetCore.Components`:
+  - `[SupplyParameterFromForm]`
+  - `[SupplyParameterFromQuery]`
+* Added the constructor and explicit cast operator of `Microsoft.AspNetCore.Components.MarkupString` as an `html-injection` sink. This will help catch cross-site scripting resulting from using `MarkupString`. 
+* Added flow summaries for the `Microsoft.AspNetCore.Mvc.Controller::View` method.
+* The data flow library has been updated to track types in a slightly different way: The type of the tainted data (which may be stored into fields, etc.) is tracked more precisely, while the types of intermediate containers for nested contents is tracked less precisely. This may have a slight effect on false positives for complex flow paths.
+* The C# extractor now supports *basic* extraction of .NET 9 projects. There might be limited support for extraction of code using the new C# 13 language features.
+
 ## 4.0.0
 
 ### Breaking Changes

csharp/ql/lib/change-notes/2024-12-04-dataflow-type-pruning-tweak.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* The data flow library has been updated to track types in a slightly different way: The type of the tainted data (which may be stored into fields, etc.) is tracked more precisely, while the types of intermediate containers for nested contents is tracked less precisely. This may have a slight effect on false positives for complex flow paths.

csharp/ql/lib/change-notes/2024-12-04-dotnet9.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* The C# extractor now supports *basic* extraction of .NET 9 projects. There might be limited support for extraction of code using the new C# 13 language features.

csharp/ql/lib/change-notes/2024-12-05-aspnetcore-mvc-model.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* Added flow summaries for the `Microsoft.AspNetCore.Mvc.Controller::View` method.

csharp/ql/lib/change-notes/2024-12-12-add-markupstring-as-html-injection-sink.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* Added the constructor and explicit cast operator of `Microsoft.AspNetCore.Components.MarkupString` as an `html-injection` sink. This will help catch cross-site scripting resulting from using `MarkupString`. 

csharp/ql/lib/change-notes/2024-12-18-blazor-attribute-sources.md

@@ -1,6 +0,0 @@
----
-category: minorAnalysis
----
-* Added `remote` flow source models for properties of Blazor components annotated with any of the following attributes from `Microsoft.AspNetCore.Components`:
-  - `[SupplyParameterFromForm]`
-  - `[SupplyParameterFromQuery]`

csharp/ql/lib/change-notes/2024-12-20-collection-params.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* C# 13: Added QL library support for *collection* like type `params` parameters.

csharp/ql/lib/change-notes/released/4.0.1.md

@@ -0,0 +1,12 @@
+## 4.0.1
+
+### Minor Analysis Improvements
+
+* C# 13: Added QL library support for *collection* like type `params` parameters.
+* Added `remote` flow source models for properties of Blazor components annotated with any of the following attributes from `Microsoft.AspNetCore.Components`:
+  - `[SupplyParameterFromForm]`
+  - `[SupplyParameterFromQuery]`
+* Added the constructor and explicit cast operator of `Microsoft.AspNetCore.Components.MarkupString` as an `html-injection` sink. This will help catch cross-site scripting resulting from using `MarkupString`. 
+* Added flow summaries for the `Microsoft.AspNetCore.Mvc.Controller::View` method.
+* The data flow library has been updated to track types in a slightly different way: The type of the tainted data (which may be stored into fields, etc.) is tracked more precisely, while the types of intermediate containers for nested contents is tracked less precisely. This may have a slight effect on false positives for complex flow paths.
+* The C# extractor now supports *basic* extraction of .NET 9 projects. There might be limited support for extraction of code using the new C# 13 language features.

csharp/ql/lib/change-notes/released/4.0.2.md

@@ -0,0 +1,7 @@
+## 4.0.2
+
+### Minor Analysis Improvements
+
+* Added extractor support for extracting implicit `ToString` calls in binary `+` expressions and string interpolation expressions.
+* The Razor source generator invocation in `build-mode:none` extraction has been changed to use relative file paths instead of absolute ones.
+* C# 13: Added extractor support and call dispatch logic (data flow) for the (negative) type parameter constraint `allows ref struct`. Added extractor support for the type parameter constraint `notnull`.

csharp/ql/lib/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 4.0.0
+lastReleaseVersion: 4.0.2

csharp/ql/lib/ext/Microsoft.AspNetCore.Http.model.yml

@@ -0,0 +1,6 @@
+extensions:
+  - addsTo:
+      pack: codeql/csharp-all
+      extensible: summaryModel
+    data:
+      - ["Microsoft.AspNetCore.Http", "PathString", True, "ToString", "()", "", "Argument[this]", "ReturnValue", "taint", "manual"]

csharp/ql/lib/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/csharp-all
-version: 4.0.1-dev
+version: 4.0.3-dev
 groups: csharp
 dbscheme: semmlecode.csharp.dbscheme
 extractor: csharp

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

@@ -649,11 +649,14 @@ predicate convBoxing(Type fromType, Type toType) {
 }
 
 private predicate convBoxingValueType(ValueType fromType, Type toType) {
-  toType instanceof ObjectType
-  or
-  toType instanceof DynamicType
-  or
-  toType instanceof SystemValueTypeClass
+  (
+    toType instanceof ObjectType
+    or
+    toType instanceof DynamicType
+    or
+    toType instanceof SystemValueTypeClass
+  ) and
+  not fromType.isRefLikeType()
   or
   toType = fromType.getABaseInterface+()
 }

csharp/ql/lib/semmle/code/csharp/Generics.qll

@@ -287,6 +287,12 @@ class TypeParameterConstraints extends Element, @type_parameter_constraints {
   /** Holds if these constraints include a nullable reference type constraint. */
   predicate hasNullableRefTypeConstraint() { general_type_parameter_constraints(this, 5) }
 
+  /** Holds if these constraints include a notnull type constraint. */
+  predicate hasNotNullTypeConstraint() { general_type_parameter_constraints(this, 6) }
+
+  /** Holds if these constraints include a `allows ref struct` constraint. */
+  predicate hasAllowRefLikeTypeConstraint() { general_type_parameter_constraints(this, 7) }
+
   /** Gets a textual representation of these constraints. */
   override string toString() { result = "where " + this.getTypeParameter().getName() + ": ..." }
 

csharp/ql/lib/semmle/code/csharp/PrintAst.qll

@@ -32,7 +32,9 @@ private predicate shouldPrint(Element e, Location l) {
 }
 
 private predicate isImplicitExpression(ControlFlowElement element) {
-  element.(Expr).isImplicit() and
+  // Include compiler generated cast expressions and `ToString` calls if
+  // they wrap actual source expressions.
+  element.(Expr).stripImplicit().isImplicit() and
   not element instanceof CastExpr and
   not element.(OperatorCall).getTarget() instanceof ImplicitConversionOperator and
   not element instanceof ElementInitializer

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

@@ -48,6 +48,13 @@ class Type extends Member, TypeContainer, @type {
 
   /** Holds if this type is a value type, or a type parameter that is a value type. */
   predicate isValueType() { none() }
+
+  /**
+   * Holds if this type is a ref like type.
+   *
+   * Only `ref struct` types are considered ref like types.
+   */
+  predicate isRefLikeType() { none() }
 }
 
 pragma[nomagic]
@@ -704,8 +711,12 @@ class Enum extends ValueType, @enum_type {
  * ```
  */
 class Struct extends ValueType, @struct_type {
-  /** Holds if this `struct` has a `ref` modifier. */
-  predicate isRef() { this.hasModifier("ref") }
+  /**
+   * DEPRECATED: Use `instanceof RefStruct` instead.
+   *
+   * Holds if this `struct` has a `ref` modifier.
+   */
+  deprecated predicate isRef() { this.hasModifier("ref") }
 
   /** Holds if this `struct` has a `readonly` modifier. */
   predicate isReadonly() { this.hasModifier("readonly") }
@@ -713,6 +724,23 @@ class Struct extends ValueType, @struct_type {
   override string getAPrimaryQlClass() { result = "Struct" }
 }
 
+/**
+ * A `ref struct`, for example
+ *
+ * ```csharp
+ * ref struct S {
+ *  ...
+ * }
+ * ```
+ */
+class RefStruct extends Struct {
+  RefStruct() { this.hasModifier("ref") }
+
+  override string getAPrimaryQlClass() { result = "RefStruct" }
+
+  override predicate isRefLikeType() { any() }
+}
+
 /**
  * A `record struct`, for example
  * ```csharp

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

@@ -522,16 +522,21 @@ module Gvn {
 
 /** Provides definitions related to type unification. */
 module Unification {
-  /** A type parameter that is compatible with any type. */
+  /** A type parameter that is compatible with any type except `ref struct`. */
   class UnconstrainedTypeParameter extends TypeParameter {
-    UnconstrainedTypeParameter() { not exists(getATypeConstraint(this)) }
+    UnconstrainedTypeParameter() {
+      not exists(getATypeConstraint(this)) and not exists(getANegativeTypeConstraint(this))
+    }
   }
 
   /** A type parameter that is constrained. */
   class ConstrainedTypeParameter extends TypeParameter {
     int constraintCount;
 
-    ConstrainedTypeParameter() { constraintCount = strictcount(getATypeConstraint(this)) }
+    ConstrainedTypeParameter() {
+      constraintCount = count(getATypeConstraint(this)) + count(getANegativeTypeConstraint(this)) and
+      constraintCount > 0
+    }
 
     /**
      * Holds if this type parameter is unifiable with type `t`.
@@ -559,7 +564,7 @@ module Unification {
     bindingset[this]
     pragma[inline_late]
     override predicate unifiable(Type t) {
-      exists(TTypeParameterConstraint ttc | ttc = getATypeConstraint(this) |
+      forall(TTypeParameterConstraint ttc | ttc = getATypeConstraint(this) |
         ttc = TRefTypeConstraint() and
         t.isRefType()
         or
@@ -567,13 +572,14 @@ module Unification {
         t.isValueType()
         or
         typeConstraintUnifiable(ttc, t)
-      )
+      ) and
+      (t.isRefLikeType() implies getANegativeTypeConstraint(this) = TAllowRefTypeConstraint())
     }
 
     bindingset[this]
     pragma[inline_late]
     override predicate subsumes(Type t) {
-      exists(TTypeParameterConstraint ttc | ttc = getATypeConstraint(this) |
+      forall(TTypeParameterConstraint ttc | ttc = getATypeConstraint(this) |
         ttc = TRefTypeConstraint() and
         t.isRefType()
         or
@@ -581,7 +587,8 @@ module Unification {
         t.isValueType()
         or
         typeConstraintSubsumes(ttc, t)
-      )
+      ) and
+      (t.isRefLikeType() implies getANegativeTypeConstraint(this) = TAllowRefTypeConstraint())
     }
   }
 
@@ -603,7 +610,8 @@ module Unification {
         t.isValueType()
         or
         typeConstraintUnifiable(ttc, t)
-      )
+      ) and
+      (t.isRefLikeType() implies getANegativeTypeConstraint(this) = TAllowRefTypeConstraint())
     }
 
     bindingset[this]
@@ -617,7 +625,8 @@ module Unification {
         t.isValueType()
         or
         typeConstraintSubsumes(ttc, t)
-      )
+      ) and
+      (t.isRefLikeType() implies getANegativeTypeConstraint(this) = TAllowRefTypeConstraint())
     }
   }
 
@@ -632,6 +641,9 @@ module Unification {
         not t instanceof TypeParameter
       }
 
+    cached
+    newtype TTypeParameterNegativeConstraint = TAllowRefTypeConstraint()
+
     cached
     TTypeParameterConstraint getATypeConstraint(TypeParameter tp) {
       exists(TypeParameterConstraints tpc | tpc = tp.getConstraints() |
@@ -650,6 +662,14 @@ module Unification {
       )
     }
 
+    cached
+    TTypeParameterNegativeConstraint getANegativeTypeConstraint(TypeParameter tp) {
+      exists(TypeParameterConstraints tpc | tpc = tp.getConstraints() |
+        tpc.hasAllowRefLikeTypeConstraint() and
+        result = TAllowRefTypeConstraint()
+      )
+    }
+
     cached
     predicate typeConstraintUnifiable(TTypeConstraint ttc, Type t) {
       exists(Type t0 | ttc = TTypeConstraint(t0) | implicitConversionRestricted(t, t0))

csharp/ql/lib/semmle/code/csharp/commons/Constants.qll

@@ -43,7 +43,7 @@ predicate isConstantComparison(ComparisonOperation co, boolean b) {
 private module ConstantComparisonOperation {
   private import semmle.code.csharp.commons.ComparisonTest
 
-  private SimpleType convertedType(Expr expr) { result = expr.stripImplicitCasts().getType() }
+  private SimpleType convertedType(Expr expr) { result = expr.stripImplicit().getType() }
 
   private int maxValue(Expr expr) {
     if convertedType(expr) instanceof IntegralType and exists(expr.getValue())

csharp/ql/lib/semmle/code/csharp/commons/Strings.qll

@@ -44,11 +44,11 @@ class ImplicitToStringExpr extends Expr {
     )
     or
     exists(AddExpr add, Expr o | o = add.getAnOperand() |
-      o.stripImplicitCasts().getType() instanceof StringType and
-      this = add.getOtherOperand(o)
+      o.stripImplicit().getType() instanceof StringType and
+      this = add.getOtherOperand(o).stripImplicit()
     )
     or
-    this = any(InterpolatedStringExpr ise).getAnInsert()
+    this = any(InterpolatedStringExpr ise).getAnInsert().stripImplicit()
   }
 }
 

csharp/ql/lib/semmle/code/csharp/dataflow/Nullness.qll

@@ -526,7 +526,11 @@ class Dereference extends G::DereferenceableExpr {
         not underlyingType instanceof NullableType
       )
     ) else (
-      this = any(QualifiableExpr qe | not qe.isConditional()).getQualifier() and
+      this =
+        any(QualifiableExpr qe |
+          not qe.isConditional() and
+          not qe.(MethodCall).isImplicit()
+        ).getQualifier() and
       not this instanceof ThisAccess and
       not this instanceof BaseAccess and
       not this instanceof TypeAccess

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

@@ -703,7 +703,7 @@ module LocalFlow {
       or
       t = any(TypeParameter tp | not tp.isValueType())
       or
-      t.(Struct).isRef()
+      t.isRefLikeType()
     ) and
     not exists(getALastEvalNode(result))
   }

csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll

@@ -14,8 +14,14 @@ private import semmle.code.csharp.Unification
 private import semmle.code.csharp.dataflow.internal.ExternalFlow
 
 module Input implements InputSig<Location, DataFlowImplSpecific::CsharpDataFlow> {
+  private import codeql.util.Void
+
   class SummarizedCallableBase = UnboundCallable;
 
+  class SourceBase = Void;
+
+  class SinkBase = Void;
+
   predicate neutralElement(SummarizedCallableBase c, string kind, string provenance, boolean isExact) {
     interpretNeutral(c, kind, provenance) and
     // isExact is not needed for C#.
@@ -176,12 +182,22 @@ private module TypesInput implements Impl::Private::TypesInputSig {
       result.asGvnType() = Gvn::getGlobalValueNumber(dt.getDelegateType().getReturnType())
     )
   }
+
+  DataFlowType getSourceType(Input::SourceBase source, Impl::Private::SummaryComponent sc) {
+    none()
+  }
+
+  DataFlowType getSinkType(Input::SinkBase sink, Impl::Private::SummaryComponent sc) { none() }
 }
 
 private module StepsInput implements Impl::Private::StepsInputSig {
   DataFlowCall getACall(Public::SummarizedCallable sc) {
     sc = viableCallable(result).asSummarizedCallable()
   }
+
+  Node getSourceNode(Input::SourceBase source, Impl::Private::SummaryComponent sc) { none() }
+
+  Node getSinkNode(Input::SinkBase sink, Impl::Private::SummaryComponent sc) { none() }
 }
 
 module SourceSinkInterpretationInput implements

csharp/ql/lib/semmle/code/csharp/dispatch/Dispatch.qll

@@ -857,7 +857,7 @@ private module Internal {
     private predicate hasDynamicArg(int i, Type argumentType) {
       exists(Expr argument |
         argument = this.getArgument(i) and
-        argument.stripImplicitCasts().getType() instanceof DynamicType and
+        argument.stripImplicit().getType() instanceof DynamicType and
         argumentType = getAPossibleType(argument, _)
       )
     }

csharp/ql/lib/semmle/code/csharp/exprs/Call.qll

@@ -281,6 +281,10 @@ class MethodCall extends Call, QualifiableExpr, LateBindableExpr, @method_invoca
       result = this.getArgument(i - 1)
     else result = this.getArgument(i)
   }
+
+  override Expr stripImplicit() {
+    if this.isImplicit() then result = this.getQualifier().stripImplicit() else result = this
+  }
 }
 
 /**

csharp/ql/lib/semmle/code/csharp/exprs/Expr.qll

@@ -82,10 +82,18 @@ class Expr extends ControlFlowElement, @expr {
   Expr stripCasts() { result = this }
 
   /**
+   * DEPRECATED: Use `stripImplicit` instead.
+   *
    * Gets an expression that is the result of stripping (recursively) all
    * implicit casts from this expression, if any.
    */
-  Expr stripImplicitCasts() { result = this }
+  deprecated Expr stripImplicitCasts() { result = this.stripImplicit() }
+
+  /**
+   * Gets an expression that is the result of stripping (recursively) all
+   * implicit casts and implicit ToString calls from this expression, if any.
+   */
+  Expr stripImplicit() { result = this }
 
   /**
    * Gets the explicit parameter name used to pass this expression as an
@@ -714,8 +722,8 @@ class Cast extends Expr {
 
   override Expr stripCasts() { result = this.getExpr().stripCasts() }
 
-  override Expr stripImplicitCasts() {
-    if this.isImplicit() then result = this.getExpr().stripImplicitCasts() else result = this
+  override Expr stripImplicit() {
+    if this.isImplicit() then result = this.getExpr().stripImplicit() else result = this
   }
 }