[csharp] need help with taint propagation

[Hug0Vincent]

Hi, I don't understand why my taint is not propagating. I have 3 test cases that look similar, but it only works for one. Here is my C# code:

protected FullyInstrumentedType(SerializationInfo info, StreamingContext context)
        {
            Console.WriteLine("Deserialization constructor");
            Data = info.GetString("Data");

            //1
            BinaryFormatter binaryFormatter = new BinaryFormatter();
            using (MemoryStream memoryStream = new MemoryStream(Convert.FromBase64String(this.Data)))
            {
                binaryFormatter.Deserialize(memoryStream);
            }

            //2
            byte[] a = Convert.FromBase64String(this.Data);
            MemoryStream stream = null;
            stream =  new MemoryStream(a);
            (new BinaryFormatter()).Deserialize(stream);

            //3
            byte[] b = Encoding.UTF8.GetBytes(this.Data);
            MemoryStream ms = null;
            ms = new MemoryStream(b);
            (new BinaryFormatter()).Deserialize(ms);

In the first scenario, the taint halts at the FromBase64String call, whereas in the third scenario, it persists until the Deserialize call, which aligns with my expectations. This observation was confirmed through a 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 semmle.code.csharp.serialization.Serialization
import PartialFlow::PartialPathGraph
import libs.Source

private module MyConfig implements DataFlow::ConfigSig {
  
  predicate isSource(DataFlow::Node mysrc) {
      mysrc.asParameter().getCallable() instanceof SerializationConstructor and
      ( mysrc.asParameter().getCallable().hasName("ClaimsPrincipal") or 
        mysrc.asParameter().getCallable().hasName("FullyInstrumentedType")
      )
    
  }

  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
    any(SerializationInfoGetTaintStep s).step(node1, node2) 
  }

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

/**
 * We want to propagate output of SerializationInfo:
 * 
 *  Data = info.GetString("Data");
 */
class SerializationInfoGetTaintStep extends GadgetAdditionalTaintStep {
  override predicate step(DataFlow::Node fromNode, DataFlow::Node toNode) {
    exists(MethodCall mc, Method m |
      mc.getTarget() = m and
      m.getName().matches("Get%") and
      m.getDeclaringType().hasFullyQualifiedName("System.Runtime.Serialization", "SerializationInfo") and

      // Taint flows from the qualifier (info) to the call result
      fromNode.asExpr() = mc.getQualifier() and
      toNode.asExpr() = mc
    )
  }
}

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"

Based on this it should propagate the taint. What am I missing here ?

Thank you.

[Hug0Vincent]

Using the same query on a different Codeql database yield the same problem on another method:

And again there is a summary step for this method here. So the problem might be somewhere else, I don't understand

[Hug0Vincent]

Oh no here there is a neutral step for this method, I don't understand why there is a difference between the 2 method signatures.

[Hug0Vincent]

I think that in my first example the difference is in the way the return value is tainted. For GetBytes it's the ReturnValue (here) and for FromBase64String (here) it's ReturnValue.Element. What's the difference ?

[Hug0Vincent]

I have another example here where the taint stop with the same query:

I don't know which is responsible here or here. I'm probably missing something and hope someone can explain it, thank you :)