Skip to main content
Information Security

Return to Answer

Clarified and refactored the code examples
Source Link
CBHacking
CBHacking
  • 52.3k
  • 3
  • 97
  • 140

Calling JSON.stringify on any valid JS object is always safe, as is passing the result of that call to JSON.parse. Stringification will automatically escape any characters that need it; no need to do so manually. Therefore, one way to do this safely (if overcautiously) is the following:

let msg = 'For input string: ' + request.UserInput;
let input = {"id": "34245425", "field": false, "information": [{"message": nullmsg}]};
input.information[0].message = 'For input string: ' + request.UserInput;
let inputStr = JSON.stringify(input);

This is always going to be safe, because you're not mixing things that are strings (the value 'For input string: ', the user content) with things that aren't (JSON or the object itself); you build a string, and then use the language features to add it to the object and built-in APIs to convert that object to (and presumably later from) a JSON string. It is safe to combine the first two lines into one, ... "information": [{"message": "For input string: " + request.UserInput}]}; because the JS parser will still evaluate the string concatenation expression before assigning it to the message property, but I spread it out for clarity. This one is safe to do anything with.

  • Remove the seemingly-redundant "For user input: " so the second line of the block above just becomes input.information[0].message = request.UserInput; and the first line goes away entirely.
  • Nest the message subfields in their own objects, so it becomesthe first two lines become input.information[0].message = {"inputString": request.UserInput}; or similar.

As a general rule, if you find yourself trying to write an XSS filter, or indeed any kind of sanitizer for a common format, you're doing at least one thing wrong. It might just be "not using a common, well-tested library", but it's reasonably likely to be "not using language or framework features that render the whole question moot" or "using a design where this is even necessary".

EDIT: Clarified the code examples to highlight the over-cautious approach

Calling JSON.stringify on any valid JS object is always safe, as is passing the result of that call to JSON.parse. Stringification will automatically escape any characters that need it; no need to do so manually. Therefore, one way to do this safely is the following:

let input = {"id": "34245425", "field": false, "information": [{"message": null}]};
input.information[0].message = 'For input string: ' + request.UserInput;
let inputStr = JSON.stringify(input);

This is always going to be safe, because you're not mixing things that are strings (the value 'For input string: ', the user content) with things that aren't (JSON or the object itself); you build a string, and then use the language features to add it to the object and built-in APIs to convert that object to (and presumably later from) a JSON string. This one is safe to do anything with.

  • Remove the seemingly-redundant "For user input: " so the second line of the block above just becomes input.information[0].message = request.UserInput;
  • Nest the message subfields in their own objects, so it becomes input.information[0].message = {"inputString": request.UserInput}; or similar.

As a general rule, if you find yourself trying to write an XSS filter, or indeed any kind of sanitizer for a common format, you're doing at least one thing wrong. It might just be "not using a common, well-tested library", but it's reasonably likely to be "not using language or framework features that render the whole question moot" or "using a design where this is even necessary".

Calling JSON.stringify on any valid JS object is always safe, as is passing the result of that call to JSON.parse. Stringification will automatically escape any characters that need it; no need to do so manually. Therefore, one way to do this safely (if overcautiously) is the following:

let msg = 'For input string: ' + request.UserInput;
let input = {"id": "34245425", "field": false, "information": [{"message": msg}]};
let inputStr = JSON.stringify(input);

This is always going to be safe, because you're not mixing things that are strings (the value 'For input string: ', the user content) with things that aren't (JSON or the object itself); you build a string, and then use the language features to add it to the object and built-in APIs to convert that object to (and presumably later from) a JSON string. It is safe to combine the first two lines into one, ... "information": [{"message": "For input string: " + request.UserInput}]}; because the JS parser will still evaluate the string concatenation expression before assigning it to the message property, but I spread it out for clarity. This one is safe to do anything with.

  • Remove the seemingly-redundant "For user input: " so the second line of the block above just becomes input.information[0].message = request.UserInput; and the first line goes away entirely.
  • Nest the message subfields in their own objects, so the first two lines become input.information[0].message = {"inputString": request.UserInput}; or similar.

As a general rule, if you find yourself trying to write an XSS filter, or indeed any kind of sanitizer for a common format, you're doing at least one thing wrong. It might just be "not using a common, well-tested library", but it's reasonably likely to be "not using language or framework features that render the whole question moot" or "using a design where this is even necessary".

EDIT: Clarified the code examples to highlight the over-cautious approach

Source Link
CBHacking
CBHacking
  • 52.3k
  • 3
  • 97
  • 140

its never inserted into the DOM as the object is stored on the sever side of the application

Well, that answers your top-level question: it's impossible to have XSS if the content isn't sent to the client, because it's a client-side attack.

Now, with that said, since your server is apparently processing JSON containing user content, you do need to worry about data corruption / content insertion / deserialization attacks (the last probably not relevant here unless you're doing something weird with that JSON). Assuming you're just storing JSON as strings (e.g. for serialization with a DB or file) and then parsing it using JSON.parse or similar, you're probably at no risk of arbitrary code execution in particular; if you try to JSON.parse('{"id": "34245425", "field": false, "information": [{"message": "For input string: ";}]}alert(document.domain)'), the result is just an error because that's not valid JSON. The only you'd be at risk for code execution is if you built that "JSON" using string concatenation, and then wrote it to a JS file and executed the JS (or passed the string to eval or equivalent, which... eval is already bad, eval anywhere near user-supplied content is crazy, never do that).

Calling JSON.stringify on any valid JS object is always safe, as is passing the result of that call to JSON.parse. Stringification will automatically escape any characters that need it; no need to do so manually. Therefore, one way to do this safely is the following:

let input = {"id": "34245425", "field": false, "information": [{"message": null}]};
input.information[0].message = 'For input string: ' + request.UserInput;
let inputStr = JSON.stringify(input);

This is always going to be safe, because you're not mixing things that are strings (the value 'For input string: ', the user content) with things that aren't (JSON or the object itself); you build a string, and then use the language features to add it to the object and built-in APIs to convert that object to (and presumably later from) a JSON string. This one is safe to do anything with.

Mind you, you can (and should) avoid string concatenation entirely. Two ways to do that:

  • Remove the seemingly-redundant "For user input: " so the second line of the block above just becomes input.information[0].message = request.UserInput;
  • Nest the message subfields in their own objects, so it becomes input.information[0].message = {"inputString": request.UserInput}; or similar.

As a general rule, if you find yourself trying to write an XSS filter, or indeed any kind of sanitizer for a common format, you're doing at least one thing wrong. It might just be "not using a common, well-tested library", but it's reasonably likely to be "not using language or framework features that render the whole question moot" or "using a design where this is even necessary".