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,23 @@
+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, "Patch", "(TStructuralType)", "", "Argument[this]", "ReturnValue", "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, "CopyChangedValues", "(TStructuralType)", "", "Argument[this]", "ReturnValue", "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, "Patch", "(T)", "", "Argument[this]", "ReturnValue", "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,132 @@
+/**
+ * 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.dataflow.FlowSteps
+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()) }
+}
+
+/**
+ * A call to `TryGetValue` on an `ODataActionParameters` dictionary copies the value
+ * of the looked-up entry into the `out` argument.
+ */
+private class ODataActionParametersTryGetValueTaintStep extends AdditionalTaintStep {
+  override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
+    exists(MethodCall mc, AssignableDefinitions::OutRefDefinition def |
+      mc.getTarget().hasName("TryGetValue") and
+      isODataActionParametersValue(mc.getQualifier()) and
+      node1.asExpr() = mc.getQualifier() and
+      def.getTargetAccess() = mc.getArgumentForName("value") and
+      node2 = DataFlow::assignableDefinitionNode(def)
+    )
+  }
+}
+
+/** 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,139 @@
+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 TryGetValueFromDictionary(ODataActionParameters parameters)
+        {
+            if (parameters.TryGetValue("Entity", out var value))
+            {
+                var entity = (BoundEntity)value;
+                Sink(entity.Name);
+            }
+        }
+
+        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 DeltaPatchReturnValue(Delta<Widget> delta, Widget original)
+        {
+            var updated = delta.Patch(original);
+            Sink(updated.Name);
+        }
+
+        void DeltaCopyChangedValuesReturnValue(Delta<Widget> delta, Widget original)
+        {
+            var updated = delta.CopyChangedValues(original);
+            Sink(updated.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);
+        }
+    }
+}

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

@@ -0,0 +1,15 @@
+| OData.cs:46:55:46:64 | parameters | OData.cs:49:18:49:23 | access to local variable entity |
+| OData.cs:46:55:46:64 | parameters | OData.cs:50:18:50:28 | access to property Name |
+| OData.cs:46:55:46:64 | parameters | OData.cs:51:18:51:31 | access to property Content |
+| OData.cs:46:55:46:64 | parameters | OData.cs:52:18:52:38 | access to property Owner |
+| OData.cs:46:55:46:64 | parameters | OData.cs:55:22:55:28 | access to property Owner |
+| OData.cs:59:55:59:64 | parameters | OData.cs:65:26:65:35 | access to property Label |
+| OData.cs:59:55:59:64 | parameters | OData.cs:72:22:72:34 | access to property Category |
+| OData.cs:76:62:76:71 | parameters | OData.cs:81:22:81:32 | access to property Name |
+| OData.cs:85:52:85:61 | parameters | OData.cs:89:18:89:28 | access to property Name |
+| OData.cs:92:39:92:43 | delta | OData.cs:95:18:95:30 | access to property Name |
+| OData.cs:98:45:98:49 | delta | OData.cs:101:18:101:23 | access to property Name |
+| OData.cs:104:50:104:54 | delta | OData.cs:107:18:107:29 | access to property Name |
+| OData.cs:110:62:110:66 | delta | OData.cs:113:18:113:29 | access to property Name |
+| OData.cs:116:67:116:71 | delta | OData.cs:119:18:119:30 | access to property Name |
+| OData.cs:122:71:122:75 | delta | OData.cs:125:18:125:23 | access to property Name |

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

@@ -0,0 +1,23 @@
+import csharp
+
+module TaintConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node n) {
+    exists(Parameter p | p = n.asParameter() |
+      p.getType().hasFullyQualifiedName("Microsoft.AspNet.OData", "ODataActionParameters")
+      or
+      p.getType().getUnboundDeclaration().hasFullyQualifiedName("Microsoft.AspNet.OData", "Delta`1")
+      or
+      p.getType().getUnboundDeclaration().hasFullyQualifiedName("System.Web.Http.OData", "Delta`1")
+    )
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(MethodCall c | c.getArgument(0) = sink.asExpr() and c.getTarget().hasName("Sink"))
+  }
+}
+
+module Taint = TaintTracking::Global<TaintConfig>;
+
+from DataFlow::Node source, DataFlow::Node sink
+where Taint::flow(source, sink)
+select source, sink

csharp/ql/test/library-tests/frameworks/OData/options

@@ -0,0 +1,3 @@
+semmle-extractor-options: /nostdlib /noconfig
+semmle-extractor-options: --load-sources-from-project:${testdir}/../../../resources/stubs/Microsoft.AspNet.OData/7.7.5/Microsoft.AspNet.OData.csproj
+semmle-extractor-options: ${testdir}/../../../resources/stubs/System.Web.Http.OData.cs

csharp/ql/test/resources/stubs/Microsoft.AspNet.OData/7.7.5/Microsoft.AspNet.OData.cs

@@ -0,0 +1,17 @@
+namespace Microsoft.AspNet.OData
+{
+    public class ODataActionParameters : System.Collections.Generic.Dictionary<string, object>
+    {
+        public ODataActionParameters() => throw null;
+    }
+
+    public class Delta<TStructuralType> where TStructuralType : class
+    {
+        public Delta() => throw null;
+        public TStructuralType GetInstance() => throw null;
+        public TStructuralType Patch(TStructuralType original) => throw null;
+        public void Put(TStructuralType original) => throw null;
+        public TStructuralType CopyChangedValues(TStructuralType original) => throw null;
+        public void CopyUnchangedValues(TStructuralType original) => throw null;
+    }
+}

csharp/ql/test/resources/stubs/Microsoft.AspNet.OData/7.7.5/Microsoft.AspNet.OData.csproj

@@ -0,0 +1,12 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <TargetFramework>net10.0</TargetFramework>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <OutputPath>bin\</OutputPath>
+    <AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="../../_frameworks/Microsoft.NETCore.App/Microsoft.NETCore.App.csproj" />
+  </ItemGroup>
+</Project>