XSS issue
dawesc opened this issue · 3 comments
https://github.com/Astn/JSON-RPC.NET/blob/master/Json-Rpc/JsonRpcProcessor.cs#L98
At the moment this field is reflected back, in addition the response is sent as text/html rather than application data so it would be good to both sanitize the request so if you send an id with some html + javascript in it then it would send it back to the browser which then renders it :(.
id
An identifier established by the Client that MUST contain a String, Number, or NULL value if included. If it is not included it is assumed to be a notification. The value SHOULD normally not be Null [1] and Numbers SHOULD NOT contain fractional parts [2]
So here we could do a few things but i think i'd recommend that we have verify the ID by checking if:
- Number then pass
- Null then pass
- String then validate with regex that can be configured but defaults to [a-zA-Z0-9-_]
What do you think? Many thanks once again for this awesome library, christopher
I'm glad your finding this library useful!
For this XSS concern, I'm not sure how to demonstrate the issue, or prove that it is an issue.
I'm also curious about how we would ensure that the ability for the ID to be a string is not compromised. I'm worried if we were to reject ID's that are valid strings because we check on some characters that we would be introducing a breaking change, and also deviating from the spec.
I speculate that this is a client side issue, not something that can really be solved server side. Any chance you could whip up an example, or help me understand why its a server side issue?
Sorry this took me a little while; here's an example project (essentially the Hello World one with the .Net XSS detection turned off) and a little test webpage:
If you spin up the website you can see what i've done is URL encoded
{"method": "helloWorld", "params": ["Hello World"], "id": "<iframe src='https://www.w3schools.com'></iframe>" }
So what happens is the id then renders that page; i've not gone to the nth degree by making it clever by doing full screening/styling/,.. just enough to show the concept that we've been told to prevent against.
I agree that having some static rule set is not going to work for everyone, but to allow maybe even just a flag saying "numeric only" that would just then stop anything like this occurring would be fantastic. Most browsers and latest .net versions attempt to detect and stop this style of XSS attacks but because people run crufty old web browsers and .Net might not manage to detect every type of XSS attack then we have been asked to do something on this by our auditor :(
Sorry so long in replying, but figured better to have a response if anyone googles this.
If your hosting this over Http, then I think the solution may be as simple as ensuring that the content type returned is application/json.
If you using the aspnet nuget for hosting, then we just need to update that project to set the proper header.