Reputation: 61351
Running HP Fortify against a C#/.NET project. The very act of feeding a JSON document from a Web source to .NET's DataContractJsonSerializer
triggers a "JSON Injection" issue in Fortify.
The following snippet would cause it:
WebClient wc = new WebClient();
string s = wc.DownloadString(SomeURL);
using (MemoryStream mst = new MemoryStream(Encoding.UTF8.GetBytes(s)))
return new DataContractJsonSerializer(typeof(SomeType)).ReadObject(mst) as SomeType;
What's Fortify's problem with deserializing JSON like that? The class has been in .NET for a while now.
EDIT: quoting the description:
- Data enters a program from an untrusted source.
In this case the data enters at DownloadString() in SF.cs at line 43.
- The data is written to a JSON stream.
In this case the JSON is written by ReadObject() in SF.cs at line 45.
Applications typically use JSON to store data or send messages. When used to store data, JSON is often treated like cached data and may potentially contain sensitive information. When used to send messages, JSON is often used in conjunction with a RESTful service and can be used to transmit sensitive information such as authentication credentials.
The semantics of JSON documents and messages can be altered if an application constructs JSON from unvalidated input. In a relatively benign case, an attacker may be able to insert extraneous elements that cause an application to throw an exception while parsing a JSON document or request. In a more serious case, such as that involving JSON injection, an attacker may be able to insert extraneous elements that allow for the predictable manipulation of business critical values within a JSON document or request. In some cases, JSON injection can lead to cross-site scripting or dynamic code evaluation.
Okay, to summarize, the risks from a malicious JSON are:
Number 1 is the desired behavior - if the JSON is grossly malformed, the application throws its hands up and stops. #2 is possible, but how do I validate for that without parsing? #3 is impossible, since the parsing logic is not JavaScript eval()
.
EDIT2: The other .NET's JSON reader, JavaScriptSerializer
, causes no errors in Fortify. Weird.
Upvotes: 0
Views: 642
Reputation: 19158
Sorry, but I have to disagree with your comment:
-> Where's the risk in parsing? Unless the parser is window.eval(), which it's not, there is no risk. If the string is malformed, the parser will throw an exception.
The actual complaint here is not of parser directly, but the values being passed to the parser. You are assuming that the data being read / passed is either JSON or not, which would lead to exception if the latter, but, you're missing an important point that there could be dangerous valid JSONs which might affect your program logic, or may lead to other serious vulnerabilities.
If you'd go through the Fortify's description about JSON Injection, there is a good point being made which you're not paying attention to:
The semantics of JSON documents and messages can be altered if an application constructs JSON from unvalidated input. In a relatively benign case, an attacker may be able to insert extraneous elements that cause an application to throw an exception while parsing a JSON document or request. In a more serious case, such as ones that involves JSON injection, an attacker may be able to insert extraneous elements that allow for the predictable manipulation of business critical values within a JSON document or request. In some cases, JSON injection can lead to cross-site scripting or dynamic code evaluation.
So, in your current code, string s = wc.DownloadString(SomeURL);
is a potential cause of problem. You should check for the sanity of this string s, before streaming it into memory. As mentioned in the paragraph above, in serious cases extraneous elements can be inserted which can affect the business logic. Obviously, this is what a static code analyser could do it for you. It's you who knows what data would be passed here, but Fortify doesn't / can't know that. After all, Fortify is a static code analyer!
Hence, if you want Fortify to not complain any further, please put a sanity check on the string like testing for cross-site scripting attack based on JSON data, etc. If you're sure that your string is going to be clean always, then you can suppress this warning. Also, just as an additional point, I think (at least to me) it is obvious to encounter these problems with the static code analyser tools.
Upvotes: 0
Reputation: 875
check to see if any dates are being exchanged, the data contact serializer for json tries to process dates as Date(...)
embedded commands, rather than iso strings
Upvotes: 1