github / github/codeql

C#: Missing modelling of Newtonsoft.Json StringEscapeHandling

Open
#15,155 0 comments 0 reactions 0 assignees View on GitHub
false-positive
Dominant language
CodeQL
Stars
10.1k
Forks
2.1k
Avg merge
2d 15h
Merged PRs (30d)
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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.