C#: Missing modelling of Newtonsoft.Json StringEscapeHandling
- Langage dominant
- CodeQL
- Étoiles
- 10.1k
- Forks
- 2.1k
- Merge moyen
- 2 j 15 h
- PR mergées (30 j)
- 141
Description
**Description of the false positive**
The C# analysis (correctly) models `Newtonsoft.Json.JsonConvert.SerializeObject` as transmitting taint from its first argument to its result. However, the second argument to the method can be a `JsonSerializerSettings` object, which in particular can specify how to escape strings (cf [`StringEscapeHandling`](https://www.newtonsoft.com/json/help/html/t_newtonsoft_json_stringescapehandling.htm)), which prevents taint from being transmitted in many cases. I don't think this is being modelled at the moment.
**Code samples or links to source code**
For example, consider [this line of code](https://github.com/gramirezl/RepoTestCodeQL/blob/bd319cdf8b92f62bbbae98f89791fc3fb206fd17/pedidos-service/Omicron.Pedidos.Services/AlmacenService/AlmacenService.cs#L67C13-L70C14):
```cs
var stringContent = new StringContent(JsonConvert.SerializeObject(dataToSend), UnicodeEncoding.UTF8, "application/json");
```
CodeQL flags this as an XSS because `dataToSend` is user controlled and `stringContent` is embedded unsafely into HTML. (I have my doubts about that alert for other reasons, but that's not the issue here).
Now let's say I rewrite this line as follow:
```cs
var settings = new JsonSerializerSettings
{
StringEscapeHandling = StringEscapeHandling.EscapeHtml
};
var stringContent = new StringContent(JsonConvert.SerializeObject(dataToSend, settings), UnicodeEncoding.UTF8, "application/json");
```
CodeQL still flags the rewritten code, but I believe that's a false positive.
Guide de contribution
Ouvrir le guide de contribution
Piste de recherche
Start by locating the C# analysis model for Newtonsoft.Json.JsonConvert.SerializeObject and compare its handling of JsonSerializerSettings.StringEscapeHandling with the supplied example. Done means the EscapeHtml setting is recognized as preventing the reported taint flow, with coverage for the rewritten serialization call.
Rédigé par le modèle d'indexation à partir du texte de l'issue.
Évaluation
- Stack technique
- csharp
- Domaine
- security
- Type d'issue
- Bug
- Difficulté
- 4/5
- Temps estimé
- 3-5 jours
- Activité
- À l'abandon
- Clarté
- Plutôt claire
- Accessibilité débutants
- 35/100