[hugo-syn]

Hello @michaelnebel

Follow-up to #21567 / #21612. Members of types bound from [WebMethod] or action-method parameters are only tainted when they are public auto-implemented properties or public fields. Real binder-populated types often don't fit that, so taint stops at the first such member and flow is lost. From my security researcher point of view I was not fully satisfied.

This PR relaxes CandidateMemberToTaint and the two transitive classes (AspNetRemoteFlowSourceMember, AspNetCoreRemoteFlowSourceMember):

• Backing-field properties: a property is accepted if its getter returns an expression reading a field and its setter assigns an expression using value to that field, for example lazy initialization or a null check. Properties that compute a value are still excluded, which keeps the isAutoImplemented guard's intent.
• Get-only collections: a plain-data property of collection type is accepted without a setter, since binders such as XmlSerializer and Json.NET call the getter and Add to the returned collection. Get-only non-collections stay untainted.
• Base types: members declared on base types of a bound type are now covered (shared helper isMemberOfBoundType).
• Constants: const fields are excluded. The extractor does not mark them static, so the transitive rule otherwise taints short.MaxValue and similar.

As noted in the discussion, tainting is type-wide, so these members are tainted wherever the type is used. The change stays narrow for that reason.

Tests extend the existing aspremote and remote flow-source tests. A change note is included.

I'm still not 100% satisfied because for more complex operations in Getter/Setter this would not work. The only way to do it properly would be with TaintTracking I think.

isPlainDataProperty is purely syntactic: the getter must return an expression that reads a field, and the setter must assign an expression using value to that field. It does not follow accessors with several statements:

public class Model
{
    private string f;

    // Tainted: the setter assigns `value` directly and the getter returns the field.
    public string Simple { get { return f; } set { f = value; } }

    // Not tainted: `value` reaches `f` only through a local variable.
    public string ViaLocal
    {
        get { var r = f; return r; }       // the returned expression is `r`, not a read of `f`
        set { var t = value; f = t; }      // the assigned expression is `t`, not `value`
    }

    // Not tainted: same problem with a branch.
    public string Conditional
    {
        get { string r; if (f != null) r = f; else r = ""; return r; }
        set { var t = value; if (t.Length > 0) f = t; }
    }

    // Tainted by a syntactic check, but only because `value` appears in the call.
    public string Wrapped { get { return f; } set { f = Normalize(value); } }
}

Local data flow (DataFlow::localFlow) would handle ViaLocal and Conditional, because it follows locals and branches. I confirmed it also handles f ?? "" and casts, which the syntactic check misses on the getter side. It would miss Wrapped, because it follows value-preserving steps only, so Trim() and Normalize() end the flow. TaintTracking::localTaint would follow those. However, TaintedMember feeds the taint-step relation, so I expect it to be circular and I haven't tried it.

I would like feedback on whether the extra precision is worth a data-flow dependency inside a candidate-member predicate.

[Copilot]

🟡 Changes recommended

The source modeling incorrectly taints copied/computed properties and non-bindable collections while missing request-data flow through properties backed by inherited fields.

3 open issues

• Allow backing fields inherited from base types · New
• Restrict getter taint to field reads and value-preserving wrappers · New
• Exclude read-only collections without in-place binding support · New

What changed in this PR

This PR broadens CodeQL’s C# ASP.NET remote-source modeling so taint can propagate through more binder-populated model members.

Changes:

• Expands property eligibility to backing-field properties and get-only collections.
• Adds inherited-member traversal and excludes constant fields.
• Extends legacy ASP.NET and ASP.NET Core test fixtures, expected results, and release documentation.

File	Description
csharp/​ql/​test/​library-tests/​dataflow/​flowsources/​remote/​remoteFlowSource.expected	Updates expected legacy source and member results.
csharp/​ql/​test/​library-tests/​dataflow/​flowsources/​remote/​RemoteFlowSource.cs	Adds legacy web-service model cases.
csharp/​ql/​test/​library-tests/​dataflow/​flowsources/​aspremote/​aspRemoteFlowSource.expected	Updates expected ASP.NET Core results.
csharp/​ql/​test/​library-tests/​dataflow/​flowsources/​aspremote/​AspRemoteFlowSource.cs	Adds ASP.NET Core model cases and behavior annotations.
csharp/​ql/​lib/​semmle/​code/​csharp/​security/​dataflow/​flowsources/​Remote.qll	Expands candidate-member rules and shares base-type traversal.
csharp/​ql/​lib/​change-notes/​2026-11-06-taint-plain-data-members.md	Documents the analysis changes.

_{Review effort: Max (auto)}

[hugo-syn]

Fixed for arrays. ArrayType is excluded from the no-public-setter branch, so string[] Names { get; } no longer qualifies. Arrays with a public setter still do. Added NamesOnlyGet as a negative test. Not fixed: read-only collections such as SynchronizedReadOnlyCollection still qualify, because they inherit from ICollection and CollectionType can't tell them apart. I'm leaving that as a known limitation.