Skip to content

C#: Taint members of types in ASP.NET user context.#21612

Open
michaelnebel wants to merge 7 commits intogithub:mainfrom
michaelnebel:csharp/legacyasptaintedmember
Open

C#: Taint members of types in ASP.NET user context.#21612
michaelnebel wants to merge 7 commits intogithub:mainfrom
michaelnebel:csharp/legacyasptaintedmember

Conversation

@michaelnebel
Copy link
Copy Markdown
Contributor

@michaelnebel michaelnebel commented Mar 30, 2026

The background of this PR is this discussion.

In this PR we

  • Taint members of types used in some of the legacy ASP contexts.
  • Taint members transitively and include fields as well.
  • Streamline the implementation for ASP.NET tainted members.

It is worth noting that tainting the members (for a given type) used in an ASP context also means that such members will be considered tainted (when the object is tainted) in other contexts. However, to streamline the approach of the legacy ASP with ASP.NET tainted members - this is the approach taken in the PR.

@hvitved : Do you believe this is still acceptable or should we strive for adding access of fields and properties as remote sources instead?

@github-actions github-actions bot added the C# label Mar 30, 2026
@michaelnebel michaelnebel force-pushed the csharp/legacyasptaintedmember branch from a4648dc to 8721d35 Compare March 31, 2026 12:20
@michaelnebel michaelnebel force-pushed the csharp/legacyasptaintedmember branch from a00f1b5 to 2d4c18e Compare April 1, 2026 10:46
@michaelnebel michaelnebel marked this pull request as ready for review April 1, 2026 11:10
@michaelnebel michaelnebel requested a review from a team as a code owner April 1, 2026 11:10
Copilot AI review requested due to automatic review settings April 1, 2026 11:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Expands the C# CodeQL remote-source modeling for legacy ASP/ASP.NET contexts by treating members (including fields) of certain ASP.NET user-context types as tainted, including transitive member tainting, and updates the associated library tests and stubs.

Changes:

  • Add member/field tainting (including transitive tainting) for types used in ASP.NET legacy contexts (e.g., action method parameters and [WebMethod] parameters).
  • Streamline tainted-member handling by reusing a shared “candidate member” definition across ASP.NET and ASP.NET Core.
  • Extend/adjust library tests (test queries, expected output, and new test cases) and add a stub for System.Web.Services.WebMethodAttribute.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
csharp/ql/test/resources/stubs/System.Web.cs Adds a stub for System.Web.Services.WebMethodAttribute to support new test coverage.
csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.ql Updates the test query shape to expose results via query predicates (including tainted members).
csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.expected Updates expected results to include tainted-member outputs and new ASP.NET web service inputs.
csharp/ql/test/library-tests/dataflow/flowsources/remote/RemoteFlowSource.cs Adds ASP.NET web service test cases with [WebMethod] parameters and member accesses.
csharp/ql/test/library-tests/dataflow/flowsources/aspremote/aspRemoteFlowSource.expected Updates expected results to include tainted fields in the ASP legacy scenario.
csharp/ql/test/library-tests/dataflow/flowsources/aspremote/AspRemoteFlowSource.cs Adjusts the test fixture to treat a public field as tainted.
csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll Implements the expanded/streamlined ASP.NET tainted-member modeling (and reuse for ASP.NET Core member candidates).
csharp/ql/lib/change-notes/2026-04-01-asp-remote-sources.md Adds a change note describing the modeling expansion.

* 1. Action method parameters.
* 2. WebMethod parameters.
*
* Note, that this also impacts uses of such types in other contexts.
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this doc comment, the phrase "Note, that" is grammatically incorrect; consider changing it to "Note that" (without the comma) for clarity.

Suggested change
* Note, that this also impacts uses of such types in other contexts.
* Note that this also impacts uses of such types in other contexts.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants