v5.3.6 Security requirement against XSS too generic

[ThunderSon]

In v5.3, requirement 5.3.6, the following is mentioned:

Verify that the application protects against JavaScript or JSON injection attacks, including for eval attacks, remote JavaScript includes, Content Security Policy (CSP) bypasses, DOM XSS, and JavaScript expression evaluation.

Now in 5.3.3, output escaping is mentioned. In 5.5.4, JSON.parse is mentioned. These 2 examples are better examples of what a requirement is than 5.3.6.

A requirement that says "Don't have vulnerabilities" is not enough. I am a security engineer and I am confused on what I should do. How do I counter CSP bypasses?! How do I do that? Isn't CSP the defense in depth to something else? What are remote JS includes? What's the attack called? Has anyone googled remote JS includes before in their lives?

1. Dangerous functions can be one thing, and have a link to JS sinks and eval
2. Remote JS includes, again, what exactly are we talking about? Loading another page's JS on top of the content, another file's content?
3. CSP bypasses: Are we saying that we are implementing a weak CSP? Or are we saying "Test the CSP you implement"? Or are we saying that we should be writing secure code? If it's the last, then CSP is not related to this requirement.
4. DOM XSS and JS expression evaluation, isn't that more "Review JS sources and the sinks they're being fed to"?

All these are being mentioned in 1 requirement.

[jmanico]

+1 I agree this needs to be fixed, thank you for this submission

[elarlang]

My first feeling is, that it's mostly duplicate to 5.3.3 and I think I have reported it somewhere long time ago, but probably it was before re-numeration and I did not find it at the moment.

First, related requirements:

• 5.3.1 Verify that output encoding is relevant for the interpreter and context required. For example, use encoders specifically for HTML values, HTML attributes, JavaScript, URL parameters, HTTP headers, SMTP, and others as the context requires, especially from untrusted inputs (e.g. names with Unicode or apostrophes, such as ねこ or O'Hara).
• 5.3.3 Verify that context-aware, preferably automated - or at worst, manual - output escaping protects against reflected, stored, and DOM based XSS.
• 5.5.4 Verify that when parsing JSON in browsers or JavaScript-based backends, JSON.parse is used to parse the JSON document. Do not use eval() to parse JSON.

Before removing requirement, we need to check, that all the meaning in requirement are covered or deprecated:

• Verify that the application protects against JavaScript
  • for me it is already covered by 5.3.1 and 5.3.3
• or JSON injection attacks
  • if we don't take this as JavaScript injection, then this one is not covered directly
• including for eval attacks
  • use JSON.parse instead, covered by 5.5.4
• remote JavaScript includes, Content Security Policy (CSP) bypasses
  • Content-Security-Policy topic, covered by 14.4.3
  • for remote JavaScript files with good purpose, it's covered by 14.2.3
• DOM XSS
  • again, covered by 5.3.3
• and JavaScript expression evaluation
  • not 100% sure on this one, but I would say it's covered with JavaScript injection or eval issues

CWE for 5.3.6 is incorrect as well:

CWE-89: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')

[ThunderSon]

I'm going to use digits for every bit you're tackling so we can discuss them easier:

1. JS injection: Agreed
2. JSON injection: then the requirements should be elevated to say "Prevent JSON injections". That can still be part of 5.5.4, but generalized with JS as an example.
3. eval attacks: I'd say this should be replaced with sinks. 5.5.4 only covers JSON injection, and not variable injection. But this affects for example setTimeout as well. Something to the lines of "Don't feed user-input into sinks/dangerous-functions".
4. JS includes, CSP: agreed with a caveat. They discuss injection prevention, but from a config perspective, so something needs to link back there. Or maybe another way to look at this?
5. DOM XSS and JS expression evaluation: agreed, should be already covered in sinks requirement and non-verified script loads.

[elarlang]

.2. JSON injection should not be part of Deserialization (5.5.4) or eval, but it's clearly 5.3 problem. I interpret "JSON injection" when someone have The Best Idea Ever to build JSON manually instead of using some built-in JSON-encoder library/function.

.4. config part I should keep away from here, even they are all related. Otherwise we need to have in this section "You must have correct Content-Type set" as well, but we have another categories for that.

.3. and 5. maybe current requirement (5.3.6) need to change to "do not execute userinput as HTML and/or JavaScript", this means DOM XSS (classical "write as text" vs "write as HTML"), eval, setTimeout etc.

Current requirement with clearly duplicate parts removed:

Verify that the application protects against JavaScript or JSON injection attacks, including for eval attacks, remote JavaScript includes, Content Security Policy (CSP) bypasses, DOM XSS, and JavaScript expression evaluation.

[elarlang]

@ThunderSon - any proposals for improving current situation or we just make quick-fix with removing clear duplicates like I showed in previous comment?

[jmanico]

This requirement is a mess. I'm deleting it now.

[elarlang]

Proposal for v4.0.3:
Verify that the application protects against JSON injection attacks, including for eval attacks, and JavaScript expression evaluation.

[jmanico]

A little wordsmithing:

Verify that the application protects against JSON injection attacks, JSON eval attacks, and JavaScript expression evaluation.

[jmanico]

PR #1097 brings this requirement back with new simplified text resolving this issue for good, I hope! :)

[elarlang]

Proposal for v4.0.3 was not to goal to bring it back for 4.* or 5.0 release. But please do not touch it now anymore before v4.0.3 is out.

[tghosth]

I am reopening this issue.

The current requirement is:

5.3.6 Verify that the application protects against JSON injection attacks, JSON eval attacks, and JavaScript expression evaluation.

• I don't know why we need to mention eval here is as well as in 5.2.4.
• I am not sure what "JavaScript expression evaluation" is but it also sounds like eval...

To me it feels like both these items can be removed although JSON injection is clearly still correct and should be specifically included.

Any comments @ThunderSon, @jmanico or @elarlang ?

[elarlang]

5.3.6 was shortened/fixed for v4.0.3. Agree, current 5.3.6 is duplicate of 5.2.4.

5.2.4 also covers JavaScript expression evaluation.

[tghosth]

@elarlang I opened #1341

[elarlang]

Current state, after update:

#	Description	L1	L2	L3	CWE
5.3.1	[MODIFIED] Verify that output encoding is relevant for the interpreter and context required. For example, use encoders specifically for HTML values, HTML attributes, JavaScript, CSS, URL parameters, HTTP headers, SMTP, and others as the context requires, especially from untrusted inputs (e.g. names with Unicode or apostrophes, such as ねこ or O'Hara). (C4)	✓	✓	✓	116
5.3.6	[MODIFIED] Verify that the application protects against JSON injection attacks. (C4)	✓	✓	✓	75

We have removed duplicates, but maybe it makes sense to put JSON into 5.3.1 list and we can get rid of 5.3.6 completely. I don't think JSON injection needs separate requirement, and if it does, we should make separate requirements for everything listed in 5.3.1.

[tghosth]

@elarlang to my mind, JSON injection is something slightly different to the items in 5.3.1 so I am not sure if it really fits there... It isn't a context for some from of XSS but rather it is the ability to inject additional JSON entities into a JSON object.

[elarlang]

Ok, I close this one.