From a1f2b815669d92bcde42d6dc77c96d23f33d7ad5 Mon Sep 17 00:00:00 2001 From: hugo-syn Date: Wed, 19 Aug 2026 15:30:47 +0200 Subject: [PATCH 1/7] C#: add OData action-parameter taint modeling Adds semmle.code.csharp.frameworks.OData, following the WCF.qll/JsonNET.qll convention: values cast, as-converted, or type-tested out of an untyped ODataActionParameters dictionary, and entities tracked by Delta (via GetInstance/Patch/Put/CopyChangedValues/CopyUnchangedValues), have no static type relationship to the action method's own parameter types, so their members aren't picked up by the existing AspNetRemoteFlowSourceMember modeling. This adds a TaintedMember for those bound types (with the same nested-type/collection recursion as AspNetRemoteFlowSourceMember), plus two AdditionalTaintStep steps for the Delta method calls, which don't fit the member-read shape TaintedMember covers. Co-Authored-By: Claude Sonnet 5 --- .../2026-08-19-odata-taint-step.md | 4 + .../internal/TaintTrackingPrivate.qll | 1 + .../semmle/code/csharp/frameworks/OData.qll | 161 ++++++++++++++++++ .../library-tests/frameworks/OData/OData.cs | 125 ++++++++++++++ .../frameworks/OData/OData.expected | 9 + .../library-tests/frameworks/OData/OData.ql | 21 +++ 6 files changed, 321 insertions(+) create mode 100644 csharp/ql/lib/change-notes/2026-08-19-odata-taint-step.md create mode 100644 csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll create mode 100644 csharp/ql/test/library-tests/frameworks/OData/OData.cs create mode 100644 csharp/ql/test/library-tests/frameworks/OData/OData.expected create mode 100644 csharp/ql/test/library-tests/frameworks/OData/OData.ql diff --git a/csharp/ql/lib/change-notes/2026-08-19-odata-taint-step.md b/csharp/ql/lib/change-notes/2026-08-19-odata-taint-step.md new file mode 100644 index 000000000000..3d0a6cbeb8ae --- /dev/null +++ b/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` (via `GetInstance`, `Patch`, `Put`, `CopyChangedValues`, and `CopyUnchangedValues`), now taint the members of the target type. diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll index 238ecab13461..418573cae8cc 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll +++ b/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 diff --git a/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll b/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll new file mode 100644 index 000000000000..899f1cbd036a --- /dev/null +++ b/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll @@ -0,0 +1,161 @@ +/** + * 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` whose + * values are cast, `as`-converted, or type-tested to arbitrary model types + * by the action method body. + * - `Delta`, a change-tracking wrapper for PATCH/PUT requests, whose + * tracked property values are exposed via `GetInstance()` 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. + */ + +private import csharp +private import semmle.code.csharp.commons.Collections +private import semmle.code.csharp.dataflow.FlowSteps +private import semmle.code.csharp.dataflow.TaintTracking +private import semmle.code.csharp.dataflow.internal.DataFlowPrivate + +/** The `ODataActionParameters` dictionary type, across OData library versions. */ +private 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") + } +} + +/** An indexer read on an `ODataActionParameters` dictionary, e.g. `parameters["CabFile"]`. */ +private class ODataActionParameterRead extends ElementAccess { + ODataActionParameterRead() { this.getQualifier().getType() instanceof ODataActionParametersClass } +} + +/** Holds if `e` may (locally) hold the value of an `ODataActionParameters` entry. */ +private predicate isODataParameterValue(Expr e) { + TaintTracking::localExprTaint(any(ODataActionParameterRead r), e) +} + +/** The generic `Delta` change-tracking class, across OData library versions. */ +private 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") + ) + } +} + +/** + * A type that a value read out of `ODataActionParameters` is cast, `as`-converted, + * or type-tested to -- directly, or wrapped in a collection (`List`, + * `IEnumerable`, arrays, ...) -- or a type that is tracked by a `Delta`. + */ +private 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) + } +} + +private class CandidateODataMember extends Member { + CandidateODataMember() { + this.isPublic() and + not this.isStatic() and + ( + this = + any(Property p | + p.isAutoImplemented() and + p.getGetter().isPublic() and + p.getSetter().isPublic() + ) + or + this = any(Field f | f.isPublic()) + ) + } +} + +/** + * 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`. + * + * 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, CandidateODataMember { + 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 + ) + ) + } +} + +/** The `Patch`, `Put`, `CopyChangedValues`, and `CopyUnchangedValues` methods on `Delta`. */ +private class DeltaMutatingMethod extends Method { + DeltaMutatingMethod() { + this.getDeclaringType() instanceof DeltaClass and + this.hasName(["Patch", "Put", "CopyChangedValues", "CopyUnchangedValues"]) + } +} + +/** + * A call to `Delta.Patch`/`Put`/`CopyChangedValues`/`CopyUnchangedValues` + * copies the changes tracked by the `Delta` receiver onto its `original` + * entity argument. + */ +private class DeltaMutatingCallTaintStep extends AdditionalTaintStep { + override predicate step(DataFlow::Node node1, DataFlow::Node node2) { + exists(MethodCall mc | + mc.getTarget().getUnboundDeclaration() instanceof DeltaMutatingMethod and + node1.asExpr() = mc.getQualifier() and + node2.(PostUpdateNode).getPreUpdateNode().asExpr() = mc.getArgument(0) + ) + } +} + +/** The `GetInstance` method on `Delta`. */ +private class DeltaGetInstanceMethod extends Method { + DeltaGetInstanceMethod() { + this.getDeclaringType() instanceof DeltaClass and + this.hasName("GetInstance") + } +} + +/** `Delta.GetInstance()` returns the tracked entity, carrying the same taint as the `Delta` itself. */ +private class DeltaGetInstanceTaintStep extends AdditionalTaintStep { + override predicate step(DataFlow::Node node1, DataFlow::Node node2) { + exists(MethodCall mc | + mc.getTarget().getUnboundDeclaration() instanceof DeltaGetInstanceMethod and + node1.asExpr() = mc.getQualifier() and + node2.asExpr() = mc + ) + } +} diff --git a/csharp/ql/test/library-tests/frameworks/OData/OData.cs b/csharp/ql/test/library-tests/frameworks/OData/OData.cs new file mode 100644 index 000000000000..ade9366f64d0 --- /dev/null +++ b/csharp/ql/test/library-tests/frameworks/OData/OData.cs @@ -0,0 +1,125 @@ +using System.Collections.Generic; + +namespace Microsoft.AspNet.OData +{ + public class ODataActionParameters : Dictionary + { + } + + public class Delta where TStructuralType : class + { + private TStructuralType instance; + + public Delta() { instance = default(TStructuralType); } + + public TStructuralType GetInstance() => instance; + + public void Patch(TStructuralType original) { } + + public void Put(TStructuralType original) { } + + public void CopyChangedValues(TStructuralType original) { } + + public void CopyUnchangedValues(TStructuralType original) { } + } +} + +namespace Test +{ + using Microsoft.AspNet.OData; + using System.Collections.Generic; + + public class FileMetadata + { + public string Author { get; set; } + } + + public class UploadedFile + { + public string FileName { get; set; } + + public string FileContent { get; set; } + + public FileMetadata Metadata { get; set; } + + public List History { get; set; } + } + + public class SubscriptionRelation + { + public string EventName { get; set; } + + public string EventType { get; set; } + } + + public class Widget + { + public string Name { get; set; } + } + + public class UnrelatedType + { + // Never reached via an ODataActionParameters/Delta cast, so this + // member must stay untainted even though `UnrelatedType` itself is + // used elsewhere in the file. + public string Name { get; set; } + } + + public class OrderController + { + void Sink(object o) { } + + void CastFromDictionary(ODataActionParameters parameters) + { + var file = (UploadedFile)parameters["CabFile"]; + Sink(file); + Sink(file.FileName); + Sink(file.FileContent); + Sink(file.Metadata.Author); + foreach (var m in file.History) + { + Sink(m.Author); + } + } + + void IsAsFromDictionary(ODataActionParameters parameters) + { + if (parameters["NewEvents"] is IEnumerable relations1) + { + foreach (var item in relations1) + { + Sink(item.EventName); + } + } + + var relations2 = parameters["NewEvents"] as IEnumerable; + foreach (var item in relations2) + { + Sink(item.EventType); + } + } + + void DeltaPatch(Delta delta, Widget original) + { + delta.Patch(original); + Sink(original.Name); + } + + void DeltaGetInstance(Delta delta) + { + var w = delta.GetInstance(); + 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); + } + } +} diff --git a/csharp/ql/test/library-tests/frameworks/OData/OData.expected b/csharp/ql/test/library-tests/frameworks/OData/OData.expected new file mode 100644 index 000000000000..dbaffab69b1d --- /dev/null +++ b/csharp/ql/test/library-tests/frameworks/OData/OData.expected @@ -0,0 +1,9 @@ +| OData.cs:72:55:72:64 | parameters | OData.cs:75:18:75:21 | access to local variable file | +| OData.cs:72:55:72:64 | parameters | OData.cs:76:18:76:30 | access to property FileName | +| OData.cs:72:55:72:64 | parameters | OData.cs:77:18:77:33 | access to property FileContent | +| OData.cs:72:55:72:64 | parameters | OData.cs:78:18:78:37 | access to property Author | +| OData.cs:72:55:72:64 | parameters | OData.cs:81:22:81:29 | access to property Author | +| OData.cs:85:55:85:64 | parameters | OData.cs:91:26:91:39 | access to property EventName | +| OData.cs:85:55:85:64 | parameters | OData.cs:98:22:98:35 | access to property EventType | +| OData.cs:102:39:102:43 | delta | OData.cs:105:18:105:30 | access to property Name | +| OData.cs:108:45:108:49 | delta | OData.cs:111:18:111:23 | access to property Name | diff --git a/csharp/ql/test/library-tests/frameworks/OData/OData.ql b/csharp/ql/test/library-tests/frameworks/OData/OData.ql new file mode 100644 index 000000000000..80d0f1d04dec --- /dev/null +++ b/csharp/ql/test/library-tests/frameworks/OData/OData.ql @@ -0,0 +1,21 @@ +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") + ) + } + + predicate isSink(DataFlow::Node sink) { + exists(MethodCall c | c.getArgument(0) = sink.asExpr() and c.getTarget().hasName("Sink")) + } +} + +module Taint = TaintTracking::Global; + +from DataFlow::Node source, DataFlow::Node sink +where Taint::flow(source, sink) +select source, sink From e88e00388effca676a02f87fc8ce712905f725ee Mon Sep 17 00:00:00 2001 From: hugo-syn Date: Wed, 19 Aug 2026 15:45:00 +0200 Subject: [PATCH 2/7] C#: make OData.qll classes public, genericize test fixtures Match WCF.qll's convention: only the TaintedMember/AdditionalTaintStep wiring classes stay private, everything else that identifies a reusable OData domain concept (ODataActionParametersClass, DeltaClass, ODataBoundType, DeltaMutatingMethod, DeltaGetInstanceMethod) is public. Also renames the test fixtures to generic placeholder names. Co-Authored-By: Claude Sonnet 5 --- .../semmle/code/csharp/frameworks/OData.qll | 44 ++++++++--------- .../library-tests/frameworks/OData/OData.cs | 48 +++++++++---------- .../frameworks/OData/OData.expected | 14 +++--- 3 files changed, 53 insertions(+), 53 deletions(-) diff --git a/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll b/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll index 899f1cbd036a..406041f04e10 100644 --- a/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll +++ b/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll @@ -17,14 +17,14 @@ * members need to be taint-tracked explicitly. */ -private import csharp +import csharp private import semmle.code.csharp.commons.Collections private import semmle.code.csharp.dataflow.FlowSteps private import semmle.code.csharp.dataflow.TaintTracking private import semmle.code.csharp.dataflow.internal.DataFlowPrivate /** The `ODataActionParameters` dictionary type, across OData library versions. */ -private class ODataActionParametersClass extends Class { +class ODataActionParametersClass extends Class { ODataActionParametersClass() { this.hasFullyQualifiedName("Microsoft.AspNet.OData", "ODataActionParameters") or this.hasFullyQualifiedName("Microsoft.AspNetCore.OData.Formatter", "ODataActionParameters") or @@ -32,8 +32,8 @@ private class ODataActionParametersClass extends Class { } } -/** An indexer read on an `ODataActionParameters` dictionary, e.g. `parameters["CabFile"]`. */ -private class ODataActionParameterRead extends ElementAccess { +/** An indexer read on an `ODataActionParameters` dictionary, e.g. `parameters["Foo"]`. */ +class ODataActionParameterRead extends ElementAccess { ODataActionParameterRead() { this.getQualifier().getType() instanceof ODataActionParametersClass } } @@ -43,7 +43,7 @@ private predicate isODataParameterValue(Expr e) { } /** The generic `Delta` change-tracking class, across OData library versions. */ -private class DeltaClass extends UnboundGenericClass { +class DeltaClass extends UnboundGenericClass { DeltaClass() { this.getNumberOfTypeParameters() = 1 and ( @@ -58,7 +58,7 @@ private class DeltaClass extends UnboundGenericClass { * or type-tested to -- directly, or wrapped in a collection (`List`, * `IEnumerable`, arrays, ...) -- or a type that is tracked by a `Delta`. */ -private class ODataBoundType extends ValueOrRefType { +class ODataBoundType extends ValueOrRefType { ODataBoundType() { exists(Cast c | isODataParameterValue(c.getExpr()) | this = c.getTargetType() or @@ -79,6 +79,22 @@ private class ODataBoundType extends ValueOrRefType { } } +/** The `Patch`, `Put`, `CopyChangedValues`, and `CopyUnchangedValues` methods on `Delta`. */ +class DeltaMutatingMethod extends Method { + DeltaMutatingMethod() { + this.getDeclaringType() instanceof DeltaClass and + this.hasName(["Patch", "Put", "CopyChangedValues", "CopyUnchangedValues"]) + } +} + +/** The `GetInstance` method on `Delta`. */ +class DeltaGetInstanceMethod extends Method { + DeltaGetInstanceMethod() { + this.getDeclaringType() instanceof DeltaClass and + this.hasName("GetInstance") + } +} + private class CandidateODataMember extends Member { CandidateODataMember() { this.isPublic() and @@ -118,14 +134,6 @@ private class ODataBoundMember extends TaintTracking::TaintedMember, CandidateOD } } -/** The `Patch`, `Put`, `CopyChangedValues`, and `CopyUnchangedValues` methods on `Delta`. */ -private class DeltaMutatingMethod extends Method { - DeltaMutatingMethod() { - this.getDeclaringType() instanceof DeltaClass and - this.hasName(["Patch", "Put", "CopyChangedValues", "CopyUnchangedValues"]) - } -} - /** * A call to `Delta.Patch`/`Put`/`CopyChangedValues`/`CopyUnchangedValues` * copies the changes tracked by the `Delta` receiver onto its `original` @@ -141,14 +149,6 @@ private class DeltaMutatingCallTaintStep extends AdditionalTaintStep { } } -/** The `GetInstance` method on `Delta`. */ -private class DeltaGetInstanceMethod extends Method { - DeltaGetInstanceMethod() { - this.getDeclaringType() instanceof DeltaClass and - this.hasName("GetInstance") - } -} - /** `Delta.GetInstance()` returns the tracked entity, carrying the same taint as the `Delta` itself. */ private class DeltaGetInstanceTaintStep extends AdditionalTaintStep { override predicate step(DataFlow::Node node1, DataFlow::Node node2) { diff --git a/csharp/ql/test/library-tests/frameworks/OData/OData.cs b/csharp/ql/test/library-tests/frameworks/OData/OData.cs index ade9366f64d0..71a5c2218b68 100644 --- a/csharp/ql/test/library-tests/frameworks/OData/OData.cs +++ b/csharp/ql/test/library-tests/frameworks/OData/OData.cs @@ -29,27 +29,27 @@ namespace Test using Microsoft.AspNet.OData; using System.Collections.Generic; - public class FileMetadata + public class EntityMetadata { - public string Author { get; set; } + public string Owner { get; set; } } - public class UploadedFile + public class BoundEntity { - public string FileName { get; set; } + public string Name { get; set; } - public string FileContent { get; set; } + public string Content { get; set; } - public FileMetadata Metadata { get; set; } + public EntityMetadata Metadata { get; set; } - public List History { get; set; } + public List Revisions { get; set; } } - public class SubscriptionRelation + public class RelatedItem { - public string EventName { get; set; } + public string Label { get; set; } - public string EventType { get; set; } + public string Category { get; set; } } public class Widget @@ -65,37 +65,37 @@ public class UnrelatedType public string Name { get; set; } } - public class OrderController + public class SampleController { void Sink(object o) { } void CastFromDictionary(ODataActionParameters parameters) { - var file = (UploadedFile)parameters["CabFile"]; - Sink(file); - Sink(file.FileName); - Sink(file.FileContent); - Sink(file.Metadata.Author); - foreach (var m in file.History) + 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.Author); + Sink(m.Owner); } } void IsAsFromDictionary(ODataActionParameters parameters) { - if (parameters["NewEvents"] is IEnumerable relations1) + if (parameters["Items"] is IEnumerable items1) { - foreach (var item in relations1) + foreach (var item in items1) { - Sink(item.EventName); + Sink(item.Label); } } - var relations2 = parameters["NewEvents"] as IEnumerable; - foreach (var item in relations2) + var items2 = parameters["Items"] as IEnumerable; + foreach (var item in items2) { - Sink(item.EventType); + Sink(item.Category); } } diff --git a/csharp/ql/test/library-tests/frameworks/OData/OData.expected b/csharp/ql/test/library-tests/frameworks/OData/OData.expected index dbaffab69b1d..a9a2796c0ca5 100644 --- a/csharp/ql/test/library-tests/frameworks/OData/OData.expected +++ b/csharp/ql/test/library-tests/frameworks/OData/OData.expected @@ -1,9 +1,9 @@ -| OData.cs:72:55:72:64 | parameters | OData.cs:75:18:75:21 | access to local variable file | -| OData.cs:72:55:72:64 | parameters | OData.cs:76:18:76:30 | access to property FileName | -| OData.cs:72:55:72:64 | parameters | OData.cs:77:18:77:33 | access to property FileContent | -| OData.cs:72:55:72:64 | parameters | OData.cs:78:18:78:37 | access to property Author | -| OData.cs:72:55:72:64 | parameters | OData.cs:81:22:81:29 | access to property Author | -| OData.cs:85:55:85:64 | parameters | OData.cs:91:26:91:39 | access to property EventName | -| OData.cs:85:55:85:64 | parameters | OData.cs:98:22:98:35 | access to property EventType | +| OData.cs:72:55:72:64 | parameters | OData.cs:75:18:75:23 | access to local variable entity | +| OData.cs:72:55:72:64 | parameters | OData.cs:76:18:76:28 | access to property Name | +| OData.cs:72:55:72:64 | parameters | OData.cs:77:18:77:31 | access to property Content | +| OData.cs:72:55:72:64 | parameters | OData.cs:78:18:78:38 | access to property Owner | +| OData.cs:72:55:72:64 | parameters | OData.cs:81:22:81:28 | access to property Owner | +| OData.cs:85:55:85:64 | parameters | OData.cs:91:26:91:35 | access to property Label | +| OData.cs:85:55:85:64 | parameters | OData.cs:98:22:98:34 | access to property Category | | OData.cs:102:39:102:43 | delta | OData.cs:105:18:105:30 | access to property Name | | OData.cs:108:45:108:49 | delta | OData.cs:111:18:111:23 | access to property Name | From 3440d4a1eb5338c78c5faf7e40fdacd53c4f8a77 Mon Sep 17 00:00:00 2001 From: hugo-syn Date: Wed, 19 Aug 2026 15:53:23 +0200 Subject: [PATCH 3/7] C#: drop redundant TaintTracking import in OData.qll import csharp already publicly imports semmle.code.csharp.dataflow.TaintTracking (and DataFlow), same as WCF.qll/JsonNET.qll rely on implicitly. Co-Authored-By: Claude Sonnet 5 --- csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll | 1 - 1 file changed, 1 deletion(-) diff --git a/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll b/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll index 406041f04e10..80917322f441 100644 --- a/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll +++ b/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll @@ -20,7 +20,6 @@ import csharp private import semmle.code.csharp.commons.Collections private import semmle.code.csharp.dataflow.FlowSteps -private import semmle.code.csharp.dataflow.TaintTracking private import semmle.code.csharp.dataflow.internal.DataFlowPrivate /** The `ODataActionParameters` dictionary type, across OData library versions. */ From 22ad8c7dc130d7dbde025590985cd17aa315cb1f Mon Sep 17 00:00:00 2001 From: hugo-syn Date: Fri, 21 Aug 2026 09:51:56 +0200 Subject: [PATCH 4/7] C#: model Delta.GetInstance() as a MaD summary instead of QL Per review feedback on #22384, replace the hand-written DeltaGetInstanceMethod/DeltaGetInstanceTaintStep taint step with a Models-as-Data summaryModel row for both the Microsoft.AspNet.OData and Microsoft.AspNetCore.OData.Deltas variants of Delta.GetInstance(). --- .../lib/ext/Microsoft.AspNet.OData.model.yml | 7 +++++++ .../semmle/code/csharp/frameworks/OData.qll | 19 ------------------- 2 files changed, 7 insertions(+), 19 deletions(-) create mode 100644 csharp/ql/lib/ext/Microsoft.AspNet.OData.model.yml diff --git a/csharp/ql/lib/ext/Microsoft.AspNet.OData.model.yml b/csharp/ql/lib/ext/Microsoft.AspNet.OData.model.yml new file mode 100644 index 000000000000..ede706dd194a --- /dev/null +++ b/csharp/ql/lib/ext/Microsoft.AspNet.OData.model.yml @@ -0,0 +1,7 @@ +extensions: + - addsTo: + pack: codeql/csharp-all + extensible: summaryModel + data: + - ["Microsoft.AspNet.OData", "Delta", True, "GetInstance", "()", "", "Argument[this]", "ReturnValue", "taint", "manual"] + - ["Microsoft.AspNetCore.OData.Deltas", "Delta", True, "GetInstance", "()", "", "Argument[this]", "ReturnValue", "taint", "manual"] diff --git a/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll b/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll index 80917322f441..4a366c12ce64 100644 --- a/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll +++ b/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll @@ -86,14 +86,6 @@ class DeltaMutatingMethod extends Method { } } -/** The `GetInstance` method on `Delta`. */ -class DeltaGetInstanceMethod extends Method { - DeltaGetInstanceMethod() { - this.getDeclaringType() instanceof DeltaClass and - this.hasName("GetInstance") - } -} - private class CandidateODataMember extends Member { CandidateODataMember() { this.isPublic() and @@ -147,14 +139,3 @@ private class DeltaMutatingCallTaintStep extends AdditionalTaintStep { ) } } - -/** `Delta.GetInstance()` returns the tracked entity, carrying the same taint as the `Delta` itself. */ -private class DeltaGetInstanceTaintStep extends AdditionalTaintStep { - override predicate step(DataFlow::Node node1, DataFlow::Node node2) { - exists(MethodCall mc | - mc.getTarget().getUnboundDeclaration() instanceof DeltaGetInstanceMethod and - node1.asExpr() = mc.getQualifier() and - node2.asExpr() = mc - ) - } -} From a44bfec092d2c71bedea0851567bd89a25f9b822 Mon Sep 17 00:00:00 2001 From: hugo-syn Date: Fri, 21 Aug 2026 09:53:09 +0200 Subject: [PATCH 5/7] C#: reuse CandidateMemberToTaint from Remote.qll in OData.qll Per review feedback on #22384, OData.qll's CandidateODataMember was an exact copy of CandidateMemberToTaint from Remote.qll. Make that class public and import it instead of duplicating it. --- .../semmle/code/csharp/frameworks/OData.qll | 20 ++----------------- .../security/dataflow/flowsources/Remote.qll | 3 ++- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll b/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll index 4a366c12ce64..3065b7127cc5 100644 --- a/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll +++ b/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll @@ -21,6 +21,7 @@ import csharp private import semmle.code.csharp.commons.Collections private import semmle.code.csharp.dataflow.FlowSteps private import semmle.code.csharp.dataflow.internal.DataFlowPrivate +private import semmle.code.csharp.security.dataflow.flowsources.Remote /** The `ODataActionParameters` dictionary type, across OData library versions. */ class ODataActionParametersClass extends Class { @@ -86,23 +87,6 @@ class DeltaMutatingMethod extends Method { } } -private class CandidateODataMember extends Member { - CandidateODataMember() { - this.isPublic() and - not this.isStatic() and - ( - this = - any(Property p | - p.isAutoImplemented() and - p.getGetter().isPublic() and - p.getSetter().isPublic() - ) - or - this = any(Field f | f.isPublic()) - ) - } -} - /** * Taint members (transitively) on types used in * 1. Casts, `as`-conversions, or type tests applied to `ODataActionParameters` values. @@ -112,7 +96,7 @@ private class CandidateODataMember extends Member { * trade-off `AspNetRemoteFlowSourceMember` (`Remote.qll`) makes for ASP.NET * action-method parameters. */ -private class ODataBoundMember extends TaintTracking::TaintedMember, CandidateODataMember { +private class ODataBoundMember extends TaintTracking::TaintedMember, CandidateMemberToTaint { ODataBoundMember() { exists(Type t, Type t0 | t = this.getDeclaringType() | (t = t0 or t = t0.(CollectionType).getElementType()) and diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll index 68c06a1828de..3eac08b1bd93 100644 --- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll +++ b/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 From 2c17dd2149fe884c18a08f82ebc136431c7016dc Mon Sep 17 00:00:00 2001 From: hugo-syn Date: Fri, 21 Aug 2026 09:53:28 +0200 Subject: [PATCH 6/7] C#: move OData test types into test/resources/stubs Per review feedback on #22384, keep the ODataActionParameters/Delta stub implementations out of the test .cs file and store them in test/resources/stubs instead, following the pattern used by other frameworks (e.g. JsonNET, Aws). The test now loads the stub project via an options file and relies on no .dll files. --- .../library-tests/frameworks/OData/OData.cs | 26 ------------------- .../frameworks/OData/OData.expected | 18 ++++++------- .../library-tests/frameworks/OData/options | 2 ++ .../7.7.5/Microsoft.AspNet.OData.cs | 19 ++++++++++++++ .../7.7.5/Microsoft.AspNet.OData.csproj | 12 +++++++++ 5 files changed, 42 insertions(+), 35 deletions(-) create mode 100644 csharp/ql/test/library-tests/frameworks/OData/options create mode 100644 csharp/ql/test/resources/stubs/Microsoft.AspNet.OData/7.7.5/Microsoft.AspNet.OData.cs create mode 100644 csharp/ql/test/resources/stubs/Microsoft.AspNet.OData/7.7.5/Microsoft.AspNet.OData.csproj diff --git a/csharp/ql/test/library-tests/frameworks/OData/OData.cs b/csharp/ql/test/library-tests/frameworks/OData/OData.cs index 71a5c2218b68..ab06179ab79a 100644 --- a/csharp/ql/test/library-tests/frameworks/OData/OData.cs +++ b/csharp/ql/test/library-tests/frameworks/OData/OData.cs @@ -1,29 +1,3 @@ -using System.Collections.Generic; - -namespace Microsoft.AspNet.OData -{ - public class ODataActionParameters : Dictionary - { - } - - public class Delta where TStructuralType : class - { - private TStructuralType instance; - - public Delta() { instance = default(TStructuralType); } - - public TStructuralType GetInstance() => instance; - - public void Patch(TStructuralType original) { } - - public void Put(TStructuralType original) { } - - public void CopyChangedValues(TStructuralType original) { } - - public void CopyUnchangedValues(TStructuralType original) { } - } -} - namespace Test { using Microsoft.AspNet.OData; diff --git a/csharp/ql/test/library-tests/frameworks/OData/OData.expected b/csharp/ql/test/library-tests/frameworks/OData/OData.expected index a9a2796c0ca5..fad59d4a9a9b 100644 --- a/csharp/ql/test/library-tests/frameworks/OData/OData.expected +++ b/csharp/ql/test/library-tests/frameworks/OData/OData.expected @@ -1,9 +1,9 @@ -| OData.cs:72:55:72:64 | parameters | OData.cs:75:18:75:23 | access to local variable entity | -| OData.cs:72:55:72:64 | parameters | OData.cs:76:18:76:28 | access to property Name | -| OData.cs:72:55:72:64 | parameters | OData.cs:77:18:77:31 | access to property Content | -| OData.cs:72:55:72:64 | parameters | OData.cs:78:18:78:38 | access to property Owner | -| OData.cs:72:55:72:64 | parameters | OData.cs:81:22:81:28 | access to property Owner | -| OData.cs:85:55:85:64 | parameters | OData.cs:91:26:91:35 | access to property Label | -| OData.cs:85:55:85:64 | parameters | OData.cs:98:22:98:34 | access to property Category | -| OData.cs:102:39:102:43 | delta | OData.cs:105:18:105:30 | access to property Name | -| OData.cs:108:45:108:49 | delta | OData.cs:111:18:111:23 | access to property Name | +| 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:39:76:43 | delta | OData.cs:79:18:79:30 | access to property Name | +| OData.cs:82:45:82:49 | delta | OData.cs:85:18:85:23 | access to property Name | diff --git a/csharp/ql/test/library-tests/frameworks/OData/options b/csharp/ql/test/library-tests/frameworks/OData/options new file mode 100644 index 000000000000..357763232dbe --- /dev/null +++ b/csharp/ql/test/library-tests/frameworks/OData/options @@ -0,0 +1,2 @@ +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 diff --git a/csharp/ql/test/resources/stubs/Microsoft.AspNet.OData/7.7.5/Microsoft.AspNet.OData.cs b/csharp/ql/test/resources/stubs/Microsoft.AspNet.OData/7.7.5/Microsoft.AspNet.OData.cs new file mode 100644 index 000000000000..2f6673628748 --- /dev/null +++ b/csharp/ql/test/resources/stubs/Microsoft.AspNet.OData/7.7.5/Microsoft.AspNet.OData.cs @@ -0,0 +1,19 @@ +// This file contains auto-generated code. +// Generated from `Microsoft.AspNet.OData, Version=7.7.5.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35`. +namespace Microsoft.AspNet.OData +{ + public class ODataActionParameters : System.Collections.Generic.Dictionary + { + public ODataActionParameters() => throw null; + } + + public class Delta where TStructuralType : class + { + public Delta() => throw null; + public TStructuralType GetInstance() => throw null; + public void Patch(TStructuralType original) => throw null; + public void Put(TStructuralType original) => throw null; + public void CopyChangedValues(TStructuralType original) => throw null; + public void CopyUnchangedValues(TStructuralType original) => throw null; + } +} diff --git a/csharp/ql/test/resources/stubs/Microsoft.AspNet.OData/7.7.5/Microsoft.AspNet.OData.csproj b/csharp/ql/test/resources/stubs/Microsoft.AspNet.OData/7.7.5/Microsoft.AspNet.OData.csproj new file mode 100644 index 000000000000..2be6995cd169 --- /dev/null +++ b/csharp/ql/test/resources/stubs/Microsoft.AspNet.OData/7.7.5/Microsoft.AspNet.OData.csproj @@ -0,0 +1,12 @@ + + + net10.0 + true + bin\ + false + + + + + + From 9c3a9b48187dc8e12f517464372ff9ed5d5423d4 Mon Sep 17 00:00:00 2001 From: hugo-syn Date: Fri, 21 Aug 2026 09:53:39 +0200 Subject: [PATCH 7/7] C#: fix DeltaClass doc comment to not name a specific type parameter Per review feedback on #22384, the doc comment named the type parameter TStructuralType, but the AspNetCore variant of Delta names it T. Refer to the unbound generic as \`Delta\`1\`\` instead. --- csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll b/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll index 3065b7127cc5..936a50d6cf30 100644 --- a/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll +++ b/csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll @@ -42,7 +42,7 @@ private predicate isODataParameterValue(Expr e) { TaintTracking::localExprTaint(any(ODataActionParameterRead r), e) } -/** The generic `Delta` change-tracking class, across OData library versions. */ +/** The generic ``Delta`1`` change-tracking class, across OData library versions. */ class DeltaClass extends UnboundGenericClass { DeltaClass() { this.getNumberOfTypeParameters() = 1 and