[csharp] taint not propagated through overridden virtual methods

[Hug0Vincent]

I have a base class with a virtual method and a child type that override the virtual method. However my taint stop in the base class it does not flow to the overridden method. Is it a normal behavior ?

Here is an example, the partial dataflow start from this in the method Method from DerivedHandler

I then added this taint step:

override predicate step(DataFlow::Node fromNode, DataFlow::Node toNode) {
  exists(Call call, Callable target, int i |
    (
      (
        fromNode.asExpr() = call.getArgument(i) and
        toNode.asParameter().getCallable() = target and
        toNode.asParameter().getIndex() = i
      )or
      (
        i = 0 and
        fromNode.(DataFlowPrivate::InstanceParameterNode).getCallable(_).getACall() = call and
        toNode.(DataFlowPrivate::InstanceParameterNode).getCallable(_) = target
      )
    )and
    call.getARuntimeTarget() = target
  )
}

And it works:

I've tried with both InstanceParameterNode and regular parameter and the result is the same. My additional taint step is not very cleaned so I was wondering if this is normal and how to solve this ?

Here is the code if needed:

namespace ConsoleApp1
{
    class BaseHandler
    {
        public virtual void PrintMessage(string msg)
        {
            Console.WriteLine("Base: " + this.BeginQuery());
            this.Method2(msg);
        }

        protected virtual string BeginQuery()
        {
            return "";
        }

        protected virtual string Method2(string test)
        {
            return test;
        }
    }

    // 2. Derived class that calls base method
    class DerivedHandler : BaseHandler
    {
        public void Method(string msg)
        {
            Console.WriteLine("Derived: " + msg);

            // Call to base method
            base.PrintMessage(msg);
        }

        protected override string BeginQuery()
        {
           return this.GetType().Name;
        }

        protected override string Method2(string test)
        {
            return test;
        }
    }
}

While pasting this I also realize that if there are 2 derived class my additional step might return overridden method from the other class.

Thank you :)

[hvitved]

Hi.

Would you mind posting the query that you used as well?

Thanks

[Hug0Vincent]

Yes sure @hvitved , here is my forward partial dataflow query with the additional taint step commented:

/**
 * @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
private import semmle.code.csharp.dataflow.internal.DataFlowPrivate as DataFlowPrivate
import PartialFlow::PartialPathGraph

private module MyConfig implements DataFlow::ConfigSig {
    predicate isSource(DataFlow::Node mysrc) {
    (
        (
        mysrc.(DataFlowPrivate::InstanceParameterNode).getCallable(_).hasName(["Method"]) or
        mysrc.asParameter().getCallable().hasName(["Method"])
        )
        and
        (
        mysrc.(DataFlowPrivate::InstanceParameterNode).getCallable(_).getDeclaringType().hasName(["DerivedHandler"]) or
        mysrc.asParameter().getCallable().getDeclaringType().hasName(["DerivedHandler"])
        )
    )
    }

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

    /*
    predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
        exists(Call call, Callable target, int i |
            fromNode.asExpr() = call.getArgument(i) and
            toNode.asParameter().getCallable() = target and
            toNode.asParameter().getIndex() = i and
            call.getARuntimeTarget() = target
        ) or
        exists(Call call, Callable target |
            fromNode.(DataFlowPrivate::InstanceParameterNode).getCallable(_).getACall() = call and
            toNode.(DataFlowPrivate::InstanceParameterNode).getCallable(_) = target and
            call.getARuntimeTarget() = target
        )
    }
    */

}

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"

[hvitved]

Thanks a lot for your report; I'm working on a fix here.

[Hug0Vincent]

Thank you so much, I would be curious to know if you find new vulnerabilities with the fix, I assume that this pattern might be common 🤔

[Hug0Vincent]

Thank you !