csharp: odata lib

csharp/ql/lib/change-notes/2026-08-19-odata-taint-step.md

@@ -0,0 +1,4 @@
+---
+category: feature
+---
+* Added taint modeling for OData action parameter binding (`Microsoft.AspNet.OData`/`Microsoft.AspNetCore.OData`). Values cast, `as`-converted, or type-tested out of `ODataActionParameters`, and entities tracked by `Delta<T>` (via `GetInstance`, `Patch`, `Put`, `CopyChangedValues`, and `CopyUnchangedValues`), now taint the members of the target type.

csharp/ql/lib/ext/Microsoft.AspNet.OData.model.yml

@@ -0,0 +1,20 @@
+extensions:
+ - addsTo:
+ pack: codeql/csharp-all
+ extensible: summaryModel
+ data:
+ - ["Microsoft.AspNet.OData", "Delta<TStructuralType>", True, "GetInstance", "()", "", "Argument[this]", "ReturnValue", "taint", "manual"]
+ - ["Microsoft.AspNet.OData", "Delta<TStructuralType>", True, "Patch", "(TStructuralType)", "", "Argument[this]", "Argument[0]", "taint", "manual"]
+ - ["Microsoft.AspNet.OData", "Delta<TStructuralType>", True, "Put", "(TStructuralType)", "", "Argument[this]", "Argument[0]", "taint", "manual"]
+ - ["Microsoft.AspNet.OData", "Delta<TStructuralType>", True, "CopyChangedValues", "(TStructuralType)", "", "Argument[this]", "Argument[0]", "taint", "manual"]
+ - ["Microsoft.AspNet.OData", "Delta<TStructuralType>", True, "CopyUnchangedValues", "(TStructuralType)", "", "Argument[this]", "Argument[0]", "taint", "manual"]
+ - ["Microsoft.AspNetCore.OData.Deltas", "Delta<T>", True, "GetInstance", "()", "", "Argument[this]", "ReturnValue", "taint", "manual"]
+ - ["Microsoft.AspNetCore.OData.Deltas", "Delta<T>", True, "Patch", "(T)", "", "Argument[this]", "Argument[0]", "taint", "manual"]
+ - ["Microsoft.AspNetCore.OData.Deltas", "Delta<T>", True, "Put", "(T)", "", "Argument[this]", "Argument[0]", "taint", "manual"]
+ - ["Microsoft.AspNetCore.OData.Deltas", "Delta<T>", True, "CopyChangedValues", "(T)", "", "Argument[this]", "Argument[0]", "taint", "manual"]
+ - ["Microsoft.AspNetCore.OData.Deltas", "Delta<T>", True, "CopyUnchangedValues", "(T)", "", "Argument[this]", "Argument[0]", "taint", "manual"]
+ - ["System.Web.Http.OData", "Delta<TEntityType>", True, "GetEntity", "()", "", "Argument[this]", "ReturnValue", "taint", "manual"]
+ - ["System.Web.Http.OData", "Delta<TEntityType>", True, "Patch", "(TEntityType)", "", "Argument[this]", "Argument[0]", "taint", "manual"]
+ - ["System.Web.Http.OData", "Delta<TEntityType>", True, "Put", "(TEntityType)", "", "Argument[this]", "Argument[0]", "taint", "manual"]
+ - ["System.Web.Http.OData", "Delta<TEntityType>", True, "CopyChangedValues", "(TEntityType)", "", "Argument[this]", "Argument[0]", "taint", "manual"]
+ - ["System.Web.Http.OData", "Delta<TEntityType>", True, "CopyUnchangedValues", "(TEntityType)", "", "Argument[this]", "Argument[0]", "taint", "manual"]

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

@@ -9,6 +9,7 @@ private import semmle.code.csharp.dispatch.Dispatch
 private import semmle.code.csharp.commons.ComparisonTest
 // import `TaintedMember` definitions from other files to avoid potential reevaluation
 private import semmle.code.csharp.frameworks.JsonNET
+private import semmle.code.csharp.frameworks.OData
 private import semmle.code.csharp.frameworks.WCF
 private import semmle.code.csharp.security.dataflow.flowsources.Remote
 

csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll

@@ -0,0 +1,115 @@
+/**
+ * Provides taint modeling for `Microsoft.AspNet.OData`/`Microsoft.AspNetCore.OData`
+ * (and the older `System.Web.Http.OData`) OData action parameter binding.
+ *
+ * OData actions receive their untrusted payload in one of two shapes that
+ * bypass the usual "type used as an action-method parameter" taint modeling:
+ *
+ * - `ODataActionParameters`, an untyped `Dictionary<string, object>` whose
+ * values are cast, `as`-converted, or type-tested to arbitrary model types
+ * by the action method body.
+ * - `Delta<T>`, a change-tracking wrapper for PATCH/PUT requests, whose
+ * tracked property values are exposed via `GetInstance()` (`GetEntity()` in
+ * the older `System.Web.Http.OData`) or copied onto an existing entity via
+ * `Patch`/`Put`/`CopyChangedValues`/`CopyUnchangedValues`.
+ *
+ * In both cases the type that ends up holding the client-controlled data has
+ * no static relationship to the action method's parameter types, so its
+ * members need to be taint-tracked explicitly.
+ */
+
+import csharp
+private import semmle.code.csharp.commons.Collections
+private import semmle.code.csharp.security.dataflow.flowsources.Remote
+
+/** The `ODataActionParameters` dictionary type, across OData library versions. */
+class ODataActionParametersClass extends Class {
+ ODataActionParametersClass() {
+ this.hasFullyQualifiedName("Microsoft.AspNet.OData", "ODataActionParameters") or
+ this.hasFullyQualifiedName("Microsoft.AspNetCore.OData.Formatter", "ODataActionParameters") or
+ this.hasFullyQualifiedName("System.Web.Http.OData", "ODataActionParameters")
+ }
+}
+
+/**
+ * Holds if `e` is (or, via local flow -- e.g. an upcast to `IDictionary<string, object>`
+ * -- may hold the value of) an `ODataActionParameters` dictionary.
+ */
+private predicate isODataActionParametersValue(Expr e) {
+ exists(ParameterAccess e0 | e0.getType() instanceof ODataActionParametersClass |
+ e0 = e or DataFlow::localExprFlow(e0, e)
+ )
+}
+
+/**
+ * An indexer read on an `ODataActionParameters` dictionary, e.g. `parameters["Foo"]`
+ * (including through an upcast to a base dictionary type/interface).
+ */
+class ODataActionParameterRead extends ElementAccess {
+ ODataActionParameterRead() { isODataActionParametersValue(this.getQualifier()) }
+}
+
+/** Holds if `e` may (locally) hold the value of an `ODataActionParameters` entry. */
+private predicate isODataParameterValue(Expr e) {
+ DataFlow::localExprFlow(any(ODataActionParameterRead r), e)
+}
+
+/** The generic ``Delta`1`` change-tracking class, across OData library versions. */
+class DeltaClass extends UnboundGenericClass {
+ DeltaClass() {
+ this.getNumberOfTypeParameters() = 1 and
+ (
+ this.hasFullyQualifiedName("Microsoft.AspNet.OData", "Delta`1") or
+ this.hasFullyQualifiedName("Microsoft.AspNetCore.OData.Deltas", "Delta`1") or
+ this.hasFullyQualifiedName("System.Web.Http.OData", "Delta`1")
+ )
+ }
+}
+
+/**
+ * A type that a value read out of `ODataActionParameters` is cast, `as`-converted,
+ * or type-tested to -- directly, or wrapped in a collection (`List<T>`,
+ * `IEnumerable<T>`, arrays, ...) -- or a type that is tracked by a `Delta<T>`.
+ */
+class ODataBoundType extends ValueOrRefType {
+ ODataBoundType() {
+ exists(Cast c | isODataParameterValue(c.getExpr()) |
+ this = c.getTargetType() or
+ this = c.getTargetType().(CollectionType).getElementType() or
+ this = c.getTargetType().(ParamsCollectionType).getElementType()
+ )
+ or
+ exists(IsExpr ie, Type t |
+ isODataParameterValue(ie.getExpr()) and
+ t = ie.getPattern().(TypePatternExpr).getCheckedType()
+ |
+ this = t or
+ this = t.(CollectionType).getElementType() or
+ this = t.(ParamsCollectionType).getElementType()
+ )
+ or
+ this = any(ConstructedClass c | c.getUnboundGeneric() instanceof DeltaClass).getTypeArgument(0)
+ }
+}
+
+/**
+ * Taint members (transitively) on types used in
+ * 1. Casts, `as`-conversions, or type tests applied to `ODataActionParameters` values.
+ * 2. The type argument of a `Delta<T>`.
+ *
+ * Note that this also impacts uses of such types in other contexts, the same
+ * trade-off `AspNetRemoteFlowSourceMember` (`Remote.qll`) makes for ASP.NET
+ * action-method parameters.
+ */
+private class ODataBoundMember extends TaintTracking::TaintedMember, CandidateMemberToTaint {
+ ODataBoundMember() {
+ exists(Type t, Type t0 | t = this.getDeclaringType() |
+ (t = t0 or t = t0.(CollectionType).getElementType()) and
+ (
+ t0 = any(ODataBoundMember m).getType()
+ or
+ t0 instanceof ODataBoundType
+ )
+ )
+ }
+}

csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll

@@ -117,7 +117,8 @@ class AspNetServiceRemoteFlowSource extends AspNetRemoteFlowSource, DataFlow::Pa
  override string getSourceType() { result = "ASP.NET web service input" }
 }
 
-private class CandidateMemberToTaint extends Member {
+/** A public, non-static, auto-implemented property or field, candidate for taint-tracking. */
+class CandidateMemberToTaint extends Member {
  CandidateMemberToTaint() {
  this.isPublic() and
  not this.isStatic() and

csharp/ql/test/library-tests/frameworks/OData/OData.cs

@@ -0,0 +1,123 @@
+namespace Test
+{
+ using Microsoft.AspNet.OData;
+ using System.Collections.Generic;
+
+ public class EntityMetadata
+ {
+ public string Owner { get; set; }
+ }
+
+ public class BoundEntity1
+ {
+ public string Name { get; set; }
+
+ public string Content { get; set; }
+
+ public EntityMetadata Metadata { get; set; }
+
+ public List<EntityMetadata> Revisions { get; set; }
+ }
+
+ public class BoundEntity2
+ {
+ public string Name { get; set; }
+ }
+
+ public class RelatedItem
+ {
+ public string Label { get; set; }
+
+ public string Category { get; set; }
+ }
+
+ public class Widget
+ {
+ public string Name { get; set; }
+ }
+
+ public class UnrelatedType
+ {
+ // Never reached via an ODataActionParameters/Delta<T> cast, so this
+ // member must stay untainted even though `UnrelatedType` itself is
+ // used elsewhere in the file.
+ public string Name { get; set; }
+ }
+
+ public class SampleController
+ {
+ void Sink(object o) { }
+
+ void CastFromDictionary(ODataActionParameters parameters)
+ {
+ var entity = (BoundEntity1)parameters["Entity"];
+ Sink(entity); // $ hasTaintFlow=line:51
+ Sink(entity.Name); // $ hasTaintFlow=line:51
+ Sink(entity.Content); // $ hasTaintFlow=line:51
+ Sink(entity.Metadata.Owner); // $ hasTaintFlow=line:51
+ foreach (var m in entity.Revisions)
+ {
+ Sink(m.Owner); // $ hasTaintFlow=line:51
+ }
+ }
+
+ void IsAsFromDictionary(ODataActionParameters parameters)
+ {
+ if (parameters["Items"] is IEnumerable<RelatedItem> items1)
+ {
+ foreach (var item in items1)
+ {
+ Sink(item.Label); // $ hasTaintFlow=line:64
+ }
+ }
+
+ var items2 = parameters["Items"] as IEnumerable<RelatedItem>;
+ foreach (var item in items2)
+ {
+ Sink(item.Category); // $ hasTaintFlow=line:64
+ }
+ }
+
+ void UpcastThenIndex(ODataActionParameters parameters)
+ {
+ var dict = (IDictionary<string, object>)parameters;
+ var entity = (BoundEntity2)dict["Entity"];
+ Sink(entity.Name); // $ hasTaintFlow=line:81
+ }
+
+ void DeltaPatch(Delta<Widget> delta, Widget original)
+ {
+ delta.Patch(original);
+ Sink(original.Name); // $ hasTaintFlow=line:88
+ }
+
+ void DeltaGetInstance(Delta<Widget> delta)
+ {
+ var w = delta.GetInstance();
+ Sink(w.Name); // $ hasTaintFlow=line:94
+ }
+
+ void LegacyDeltaPatch(System.Web.Http.OData.Delta<Widget> delta, Widget original)
+ {
+ delta.Patch(original);
+ Sink(original.Name); // $ hasTaintFlow=line:100
+ }
+
+ void LegacyDeltaGetEntity(System.Web.Http.OData.Delta<Widget> delta)
+ {
+ var w = delta.GetEntity();
+ Sink(w.Name); // $ hasTaintFlow=line:106
+ }
+
+ void Untainted()
+ {
+ var w = new Widget();
+ w.Name = "safe";
+ Sink(w.Name);
+
+ var u = new UnrelatedType();
+ u.Name = "also safe";
+ Sink(u.Name);
+ }
+ }
+}