[dotnet] taint propagation errors / regressions

[Hug0Vincent]

Hello, I'm playing with deserilization vulnerabilities and I found an error where a test case already exist, however I think it's not properly handling the case. I just added a variable assignment and the taint stop in the constructor:

[WebMethod]publicvoidUpdateConfiguration2(stringdata){vards=newBinaryFormatter();// BADMemoryStreamms=newMemoryStream(Convert.FromBase64String(data))
ds.Deserialize(ms);// $ Alert[cs/unsafe-deserialization-untrusted-input]}

Here is the original test case:

classBadBinaryFormatter2{publicstaticobjectDeserialize(TextBoxtype,TextBoxdata){vards=newBinaryFormatter();// BADreturnds.Deserialize(newMemoryStream(Convert.FromBase64String(data.Text)));// $ Alert[cs/unsafe-deserialization-untrusted-input]}}

Some changes were made in some summary models for example in the method FromBase64Stringhere

 - ["System", "Convert", False, "FromBase64String", "(System.String)", "", "Argument[0]", "ReturnValue", "taint", "manual"]

Before it was this (ReturnValue.Element)

 - ["System", "Convert", False, "FromBase64String", "(System.String)", "", "Argument[0]", "ReturnValue.Element", "taint", "manual"]

Here is the summaryModel for MemoryStream, I tried to remove the .Element part but I get the same results:

 - ["System.IO", "MemoryStream", False, "MemoryStream", "(System.Byte[])", "", "Argument[0].Element", "Argument[this]", "taint", "manual"]

It reminds me another problem that we already discuss so I don't know if it's linked: #19911

This .Element is confusing since some methods are using it and other not. Also I don't think there is an implicit read by default but I might be wrong. Do you have any idea ?

Here is my 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
privatemodule MyConfig implements DataFlow::ConfigSig{predicateisSource(DataFlow::Nodesource){exists(Parameterp|p.hasName("data")andp.getCallable().hasName(["UpdateConfiguration2","Deserialize"])andsource.asParameter()=p)}predicateisSink(DataFlow::Nodesink){none()}predicateisAdditionalFlowStep(DataFlow::Nodenode1, DataFlow::Nodenode2){exists(MemberAccessma|// node2 is the reading of the fieldma=node2.asExpr()and// node1 is the object being accessedma.getQualifier()=node1.asExpr())}}privatemodule MyFlow = TaintTracking::Global<MyConfig>; // or DataFlow::Global<..>intexplorationLimit(){result=10}privatemodule PartialFlow = MyFlow::FlowExplorationFwd<explorationLimit/0>;
from PartialFlow::PartialPathNodesource, PartialFlow::PartialPathNodesinkwhere PartialFlow::partialFlow(source,sink, _)selectsink.getNode(),source,sink,"This node receives taint from $@.",source.getNode(),"this source"

Thank you :)

[Hug0Vincent]

My imports are missing and CodeQL didn't find MemoryStream it's my fault sorry for the spam :)

[redsun82]

no worries! Thanks for coming back to this, I was about to launch an investigation 🙂 glad you managed to sort it out!

[Hug0Vincent]

If you have time to look at this one #21567 I would appreciate :)