[dotnet] Missing Taint Flow from `[WebMethod]` Parameter Objects to Properties/Fields #21567

Hug0Vincent · 2026-03-24T16:40:39Z

Hug0Vincent
Mar 24, 2026

In ASP.NET Legacy Web Services (System.Web.Services), parameters decorated with the [WebMethod] attribute are correctly identified as RemoteFlowSource. However, the TaintTracking engine fails to propagate taint from the parameter object itself to its properties or fields during an assignment.

using System;
using System.Web.Services;
using System.Diagnostics;

namespace CodeQLRepro
{
    public class MyWrapper
    {
        public string Payload { get; set; }
    }

    public class VulnerableService : WebService
    {
        [WebMethod]
        public void ExecuteCommand(MyWrapper data)
        {
            if (data != null)
            {
                // TAINT GAP: 'data' is a RemoteFlowSource, but 'cmd' is considered clean.
                string cmd = data.Payload;

                if (!string.IsNullOrEmpty(cmd))
                {
                    // SINK: Remote Command Execution (RCE)
                    // This is NOT flagged by the standard CodeQL C# suite.
                    Process.Start("cmd.exe", "/c " + cmd);
                }
            }
        }
    }
}

When running a global taint-tracking query, the flow starts at the data parameter but terminates immediately at the property access data.Payload. The variable cmd is not marked as tainted.

Here is my partial dataflow query:

/**
 * @name Forward Partial Dataflow
 * @description Forward Partial Dataflow
 * @kind path-problem
 * @precision low
 * @problem.severity error
 * @id githubsecuritylab/forward-partial-dataflow
 * @tags template
 */

import csharp
import semmle.code.csharp.dataflow.TaintTracking
import PartialFlow::PartialPathGraph
import semmle.code.csharp.dataflow.flowsources.Remote

private module MyConfig implements DataFlow::ConfigSig {
  predicate isSource(DataFlow::Node source) {
    //exists(Parameter p |
    //  p.hasName("data") and 
    //  p.getCallable().hasName(["ExecuteCommand", "UpdateConfiguration"]) and
    //  source.asParameter() = p
    //)
    source instanceof RemoteFlowSource
  }

  predicate isSink(DataFlow::Node sink) { none() }

  //predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
  //  exists(MemberAccess ma |
  //    // node2 is the reading of the field
  //    ma = node2.asExpr() and
  //    // node1 is the object being accessed
  //    ma.getQualifier() = node1.asExpr()
  //  )
  //}
}

private module MyFlow = TaintTracking::Global<MyConfig>; // or DataFlow::Global<..>

int explorationLimit() { result = 10 }

private module PartialFlow = MyFlow::FlowExplorationFwd<explorationLimit/0>;

from PartialFlow::PartialPathNode source, PartialFlow::PartialPathNode sink
where PartialFlow::partialFlow(source, sink, _)
select sink.getNode(), source, sink, "This node receives taint from $@.", source.getNode(),
  "this source"

Adding the isAdditionalFlowStep resolves the issue. Is it a normal behavior ?

For Java there is this method localAdditionalTaintExprStep, and I think it would be nice to do the same for csharp. I see references to spring in this file and I think it's a bit similar to ASP.NET in some way.

Answered by michaelnebel

Apr 22, 2026

Thank you for the input. The PR has been merged and can be found here.

View full answer

Hug0Vincent · 2026-03-26T08:58:54Z

Hug0Vincent Mar 26, 2026
Author

Hello, I think you just quoted my answer without any comment is it normal ?

Hug0Vincent · 2026-03-27T23:41:05Z

Hug0Vincent
Mar 27, 2026
Author

I think I found the issue but I don't know how to fix it, here this class does not have associated TaintedMember class that could propagate taint on properties.

I found an example for ASP.NET Core here but it's not transitive. It's possible to have something like this:

public class AddresDto
{
      public string City {get; set;}
} 

public class UserDto
{
      public AddresDto Address {get; set;}
}

Also the this.isAutoImplemented() check can be removed I think.

I tried to make a fix but it need some recursion to get a property of a property. So I think it would be easier to taint outputs of getters like in spring and Java. It's internal CodeQL code so I don't know how to approach this.

0 replies

michaelnebel · 2026-03-30T12:40:24Z

michaelnebel
Mar 30, 2026
Maintainer

Thank you for the question.

You are right,

Members are not automatically tainted - it requires some extra logic.
Only AspNetCoreRemoteFlowSource automatically taints members.

It is worth noting that all uses of types used as eg. a parameter type for an ASP.NET action method will have their members tainted with this logic. That is, introducing taint members logic can yield results other places in the code - as it is the use of the type in the ASP.NET context that decides whether we consider the members tainted (in all use-cases in a given database).

I don't think the this.isAutoImplemented() should be removed - this might lead to an over-approximation of tainted members in case there user defined setters and getters (the check is there to be as conservative as possible).
Note, that the spring and Java tainting doesn't "transitively" taint (it only taints the result when calling getters on "unsafe" types).

I will try and draft a PR for generally tainting members on types used in ASP.NET, but I am not sure that we will be able to merge it - as it might be considered a bit to controversial to introduce in general (in case we don't merge it, you are than welcome to use the code in your own query).

11 replies

Hug0Vincent Mar 31, 2026
Author

I think that the ASP.NET Core equivalent should also be affected by this change, what do you think ?

private class AspNetCoreRemoteFlowSourceMember extends TaintTracking::TaintedMember, Property {
  AspNetCoreRemoteFlowSourceMember() {
    this.getDeclaringType() = any(AspNetCoreRemoteFlowSource source).getType() and
    this.isPublic() and
    not this.isStatic() and
    this.isAutoImplemented() and
    this.getGetter().isPublic() and
    this.getSetter().isPublic()
  }
}

michaelnebel Mar 31, 2026
Maintainer

It is worth noting that all uses of types used as eg. a parameter type for an ASP.NET action method will have their members tainted with this logic. That is, introducing taint members logic can yield results other places in the code - as it is the use of the type in the ASP.NET context that decides whether we consider the members tainted (in all use-cases in a given database).

For instance (from the test case in the PR), we have

[WebMethod]
public void MyMethod(MyData data) { ... }

This means that in all other places where use the type MyData - then the members of MyData will be considered tainted (if the object itself is tainted).
If we have something like

public void M() {
  MyData d = GetTaintedMyData();
  // All properties and fields of `d` are now recursively tainted as well, because the type `MyData` is used as a parameter type in `MyMethod`
}

Even though M might be located in a completely different place in the code.

michaelnebel Mar 31, 2026
Maintainer

Btw. would it make sense to require that a type is Serializable when tainting the members?

michaelnebel Mar 31, 2026
Maintainer

I think that the ASP.NET Core equivalent should also be affected by this change, what do you think ?

private class AspNetCoreRemoteFlowSourceMember extends TaintTracking::TaintedMember, Property {
  AspNetCoreRemoteFlowSourceMember() {
    this.getDeclaringType() = any(AspNetCoreRemoteFlowSource source).getType() and
    this.isPublic() and
    not this.isStatic() and
    this.isAutoImplemented() and
    this.getGetter().isPublic() and
    this.getSetter().isPublic()
  }
}

Yes, if we end up merging it, then that is probably a good idea!

Hug0Vincent Mar 31, 2026
Author

Btw. would it make sense to require that a type is Serializable when tainting the members?

I think the requirement is only on Getter/Setters.

Even though M might be located in a completely different place in the code.

So why taint Properties and not output of Getters in this case ? How is it handled in other languages ?

michaelnebel · 2026-04-22T07:30:25Z

michaelnebel
Apr 22, 2026
Maintainer

Thank you for the input. The PR has been merged and can be found here.

3 replies

Hug0Vincent Apr 22, 2026
Author

Thank you so much for your time and for this PR 🚀

michaelnebel Apr 22, 2026
Maintainer

I really appreciate the input and sorry for the delay (other stuff came up, which caused the delay).

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

[dotnet] Missing Taint Flow from [WebMethod] Parameter Objects to Properties/Fields #21567

Uh oh!

Uh oh!

Hug0Vincent Mar 24, 2026

Replies: 4 comments · 15 replies

This comment was marked as duplicate.

Uh oh!

Hug0Vincent Mar 26, 2026 Author

Uh oh!

Hug0Vincent Mar 27, 2026 Author

Uh oh!

michaelnebel Mar 30, 2026 Maintainer

Uh oh!

Uh oh!

Hug0Vincent Mar 31, 2026 Author

Uh oh!

Uh oh!

michaelnebel Mar 31, 2026 Maintainer

Uh oh!

michaelnebel Mar 31, 2026 Maintainer

Uh oh!

michaelnebel Mar 31, 2026 Maintainer

Uh oh!

Hug0Vincent Mar 31, 2026 Author

Uh oh!

michaelnebel Apr 22, 2026 Maintainer

Uh oh!

Hug0Vincent Apr 22, 2026 Author

Uh oh!

michaelnebel Apr 22, 2026 Maintainer

Uh oh!

Hug0Vincent Apr 22, 2026 Author

[dotnet] Missing Taint Flow from `[WebMethod]` Parameter Objects to Properties/Fields #21567

Hug0Vincent
Mar 24, 2026

Replies: 4 comments 15 replies

Hug0Vincent Mar 26, 2026
Author

Hug0Vincent
Mar 27, 2026
Author

michaelnebel
Mar 30, 2026
Maintainer

Hug0Vincent Mar 31, 2026
Author

michaelnebel Mar 31, 2026
Maintainer

michaelnebel Mar 31, 2026
Maintainer

michaelnebel Mar 31, 2026
Maintainer

Hug0Vincent Mar 31, 2026
Author

michaelnebel
Apr 22, 2026
Maintainer

Hug0Vincent Apr 22, 2026
Author

michaelnebel Apr 22, 2026
Maintainer

Hug0Vincent Apr 22, 2026
Author