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) {
+  e.getType() instanceof ODataActionParametersClass
+  or
+  DataFlow::localExprFlow(any(Expr e0 | isODataActionParametersValue(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,118 @@
+namespace Test
+{
+    using Microsoft.AspNet.OData;
+    using System.Collections.Generic;
+
+    public class EntityMetadata
+    {
+        public string Owner { get; set; }
+    }
+
+    public class BoundEntity
+    {
+        public string Name { get; set; }
+
+        public string Content { get; set; }
+
+        public EntityMetadata Metadata { get; set; }
+
+        public List<EntityMetadata> Revisions { 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 = (BoundEntity)parameters["Entity"];
+            Sink(entity);
+            Sink(entity.Name);
+            Sink(entity.Content);
+            Sink(entity.Metadata.Owner);
+            foreach (var m in entity.Revisions)
+            {
+                Sink(m.Owner);
+            }
+        }
+
+        void IsAsFromDictionary(ODataActionParameters parameters)
+        {
+            if (parameters["Items"] is IEnumerable<RelatedItem> items1)
+            {
+                foreach (var item in items1)
+                {
+                    Sink(item.Label);
+                }
+            }
+
+            var items2 = parameters["Items"] as IEnumerable<RelatedItem>;
+            foreach (var item in items2)
+            {
+                Sink(item.Category);
+            }
+        }
+
+        void UpcastThenIndex(ODataActionParameters parameters)
+        {
+            IDictionary<string, object> dict = parameters;
+            var entity = (BoundEntity)dict["Entity"];
+            Sink(entity.Name);
+        }
+
+        void DeltaPatch(Delta<Widget> delta, Widget original)
+        {
+            delta.Patch(original);
+            Sink(original.Name);
+        }
+
+        void DeltaGetInstance(Delta<Widget> delta)
+        {
+            var w = delta.GetInstance();
+            Sink(w.Name);
+        }
+
+        void LegacyDeltaPatch(System.Web.Http.OData.Delta<Widget> delta, Widget original)
+        {
+            delta.Patch(original);
+            Sink(original.Name);
+        }
+
+        void LegacyDeltaGetEntity(System.Web.Http.OData.Delta<Widget> delta)
+        {
+            var w = delta.GetEntity();
+            Sink(w.Name);
+        }
+
+        void Untainted()
+        {
+            var w = new Widget();
+            w.Name = "safe";
+            Sink(w.Name);
+
+            var u = new UnrelatedType();
+            u.Name = "also safe";
+            Sink(u.Name);
+        }
+    }
+}