False positive - when json.Marshal output is used - cant result in "Potentially unsafe quoting"

[davidhadas]

Description of the false positive

An output of golang json.Marshal is apparently not at risk for unsafe quoting

Code samples or links to source code

...
learned, errMarshal := json.Marshal(guardianSpec.Learned)
...
str := fmt.Sprintf(`[{"op":"replace","path":"/spec/learned","value":%s},{"op":"replace","path":"/spec/samples","value":%d}]`, learned, guardianSpec.NumSamples)
...

URL to the alert on GitHub code scanning (optional)
knative-extensions/security-guard#236

[mbg]

Hi @davidhadas 👋🏻 thank you for reporting this!

I have looked into what's going on here and it does indeed seem like a false positive to me, so thank you very much for bringing it to our attention. Feel free to close the alert on your repository as such.

In terms of what's going on: json.Marshal is explicitly a taint source for this query, because it may return strings that contain quotes. Using those in some form of string concatenation that is sensitive to quotes would be problematic. However, our heuristic for whether it is used in such a concatenation is overly broad right now: e.g. in the case of fmt.Sprintf, we just check that the string literals to the left and right of the formatting placeholder contain quotes. However, as the quotes here are balanced, there is no risk of the injected string altering the meaning of the result.

We probably won't get to improving the heuristic here straight away, but we will update this issue once we have.

[davidhadas]

hey @mbg,
thanks for your response.
My point is that json.Marshal() quotes are always balanced - so it cant make a balanced quote become unbalanced...

So if the code embeds a json.Marshal() output, and both left and right are balanced, you should not raise an alert.

[smowton]

I don't think that's right -- if someone is trying to enclose something in quotes like "\"%s\"" and the thing embedded is a JSON object, if I could induce it to include a quoted string I'd be able to escape from the intended quoting and do the classic "; drop table students; or what-have-you. I'd need to do something to cope with the other balancing quote provided by the JSON marshalling, but my ability to escape from the enclosure is the important thing here.