Skip to content

Improve C# insecure deserialization rule

Michael Henriksen requested to merge fix/issue-429235 into main

Context

A customer reported a false-positive in the insecure deserialization rule for C#. Specifically, the rule flagged a call to the ReadObject method on an instance of DataContractSerializer as insecure. However, according to Microsoft's deserialization guide, this serializer is not considered insecure.

Improvement

The rule is transformed into a taint tracking rule. This change enables a more precise definition of data sources that could potentially be under user control, as well as precise sink definitions that exclusively detect insecure serializers. When user-controlled data sources flow into insecure sinks, it significantly increases the likelihood of detecting true positives.

It's important to note that this enhanced precision does come at a cost. The new rule may be less effective at detecting inter-procedural or inter-functional issues compared to the previous one. However, the decision was made to prioritize increased detection precision.

Semgrep Playground

https://semgrep.dev/playground/s/51ll

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/429235+

Edited by Michael Henriksen

Merge request reports