5.3.1 and 5.3.3 seem duplicate

[jmanico]

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). (C4)

5.3.3	Verify that context-aware, preferably automated - or at worst, manual - output escaping protects against reflected, stored, and DOM based XSS. (C4)

[elarlang]

related issue from past (before v4.0 release, numbers are different):
#524

[tghosth]

Gosh, I can see this has been through a few iterations and I am inclined to agree with Jim that this looks a bit like a duplication.

@elarlang do I understand correctly that the 5.3.1 is talking about generally encoding to make things render correctly whereas 5.3.3 is about encoding to specifically prevent XSS?

[sseelmann]

To me it seems the recently added 5.3.11 is redundant too:

5.3.11	[MOVED FROM 1.5.4] Verify that output encoding occurs close to or by the interpreter for which it is intended. (C4)

[elarlang]

It's not recently added, it has been there since v4.0, just moved to different number (from 1.5.4 to 5.3.11 like the label says)

Related issue: #1484

[tghosth]

I guess I need to go through the history of these requirements, @elarlang do you think they should be separate?

[elarlang]

I'm ok with 5.3.1.

Requirement 5.3.3 needs some update, but I think it's not duplicate.

Goals here (not covered by 5.3.1):

• clearly cover defence against JavaScript injection (expected defence - escaping e.g. \' OR (covered by 5.3.1) converting to unicode e.g. \u0027)
• when userinput is applyed to DOM as HTML instead of text (.createTextNode() vs document.write(), .innerHTML etc) - in theory you can fix it with encoding, but it's unnecessary as actually you should use different method to display the data.

The term XSS is just... crap, bubble and clearly deprecated (25+ years). Not a single part is true with the name (think about Stored HTML injection - no cross-site, no scripting) and we should not use it anywhere. The term Site in the name is nowadays clear term (eTLD+1) and from JavaScript execution perspective, so-called XSS is more Same-Origin Scripting. And what is DOM XSS? It can be reflected, it can be stored.

We should say instead:

• how the payload is stored or delivered (reflected, stored)
• where payload affects the application (client-side, server-side)
• in what syntax is the problem (HTML, JavaScript, JSON)
• what is the problem (injection or execution)

Summary - 5.3.3 needs more work and analyze, are all the "XSS" cases covered with other requirements or not.

[tghosth]

@elarlang

Circling back to this issue.

I feel like Javascript encoding is covered by 5.3.1 which clearly talks about context relevant encoding for multiple locations including Javascript:

#	Description	L1	L2	L3	CWE
5.3.1	[MODIFIED] Verify that output encoding for an HTTP response/HTML document/XML document is relevant for the context required, such as encoding the relevant characters for HTML elements, HTML attributes, HTML comments, JavaScript, CSS, or HTTP headers, to avoid changing the message or document structure.	✓	✓	✓	116

As such, I wonder whether we should rework 5.3.3 to focus on your second point, safe sinks.

For example:

#	Description	L1	L2	L3	CWE
5.3.3	[MODIFIED] Verify that safe functions are used to write potentially dynamic content to a web page to avoid the risk of Cross-site Scripting (XSS) vulnerabilities.	✓	✓	✓	79

Whereas 5.3.1 is a little more generic, this gives us a standard and actionable XSS prevention requirement.

What do you think?

[elarlang]

I think 5.3.3 should concentrate for defense against JavaScript injection.

Your proposed new requirement as 5.3.3 should be separate requirement (I need to check, maybe we have it covered but need to make it more clear, similar to #1998). It is against HTML execution by mistakes in front-end JavaScript due to incorrect functions in use. Can cover also with front end eval() calls.

[tghosth]

I think 5.3.3 should concentrate for defense against JavaScript injection.

To be clear, you are talking about injection into any context which results in client-side JavaScript being executed? (i.e. what people usually mean when they talk about XSS?)

Your proposed new requirement as 5.3.3 should be separate requirement (I need to check, maybe we have it covered but need to make it more clear, similar to #1998). It is against HTML execution by mistakes in front-end JavaScript due to incorrect functions in use. Can cover also with front end eval() calls.

Not sure I agree that it should be a separate requirement, is it not just another mechanism to prevent JavaScript injection/XSS?

@elarlang

[jmanico]

Not sure I agree that it should be a separate requirement, is it not just another mechanism to prevent JavaScript injection/XSS?

XSS is not about just JavaScript injection. HTML, SVG, JavaScript and HTML can all cause XSS-centric injection. I know many of you including @tghosth and @elarlang know this already and I mean no disrespect. I just want to explain why I do not think we need a separate requirement for just JavaScript injection.

[tghosth]

Discussed the following draft but @elarlang is going to think about it more

[MODIFIED] Verify that output encoding or escaping are used to write potentially dynamic content to HTML, SVG and JavaScript documents to avoid the risk of Cross-site Scripting (XSS) vulnerabilities.

[elarlang]

We have "make the output correctly requirements"

• HTML injection / encoding - covered by 5.3.1
• JavaScript injection / escaping or encoding - let's say it is targeted by 5.3.3, but the requirement itself requires some work

What we need to address separately is when incorrect method is used for displaying the data to the output - e. g. applied as HTML instead (document.write(), .innerHTML) of as text (.createTextNode()).

We have requirement 5.2.4

V5.2.4 Verify that the application avoids the use of eval() or other dynamic code execution features. Where there is no alternative, any user input being included must be sanitized or sandboxed before being executed.

I would say it is not covering this issue, as the expected defense in this case is not to modify the content to be safe for given usage, but to use correct method for displaying it.

So my proposal is:

• to modify current 5.3.3 to address JavaScript related dynamic content building (defense methods encoding and escaping)
• add new separate requirement to "V50.5 Unintended Content Interpretation" to address incorrect method problem
  • alternative is to use more general "avoid content execution" somewhere "V10"

[jmanico]

@elar 5.3.3 suggestion

5.3.3: Verify that context-aware output encoding or escaping is used, preferably automated but at least manual, when writing dynamic content to HTML, SVG, and inline JavaScript values to prevent reflected, stored, and DOM-based Cross Site Scripting (XSS) attacks.

[elarlang]

This 5.3.3 is just full of useless noise. It should just say that "escape or encode for JavaScript".

If we do that, we should remove the word "JavaScript" from 5.3.1 and I think it can be merged with 5.3.6 (JSON injection attacks).

[jmanico]

Less noise:

5.3.3: Ensure that dynamic content written to HTML, SVG, and inline JavaScript is properly encoded or escaped to prevent XSS attacks. This should be automated when possible.

[jmanico]

I'd like to keep 5.3.3 separate from 5.3.1 since they really do handle different risks at different parts of software architecture.

[elarlang]

I proposed (see #1195 (comment)) to have 3 requirements

• current 5.3.1, without JavaScript
• current 5.3.3 - clearly only encoding and escaping when building JavaScript dynamically
• new requirement to address correct output method to use (e.g. text vs HTML) - in this case encoding or sanitization is not the correct defense, so current 5.3.3 is not the correct defense and can not cover that problem

[tghosth]

@elarlang let's discuss on Thursday

[tghosth]

I discussed with @elarlang the possibility of merging two of the above ideas for 5.3.3:

Verify that safe functions are used to write potentially dynamic content to a web page to avoid the risk of Cross-site Scripting (XSS) vulnerabilities.

and

Verify that output encoding or escaping are used to write potentially dynamic content to HTML, SVG and JavaScript documents to avoid the risk of Cross-site Scripting (XSS) vulnerabilities.

I agree that we need to keep this requirement as short as possible but to comprehensively cover "Cross-site Scripting" (I know Elar doesn't like this phrase :) )

new requirement to address correct output method to use (e.g. text vs HTML) - in this case encoding or sanitization is not the correct defense, so current 5.3.3 is not the correct defense and can not cover that problem

I agree with this point and it sounds like it needs to be V50 as it relates to how a particular document or fragment is rendered.

@elarlang to work on this

[tghosth]

@elarlang ping :)

[elarlang]

Attempt n+1, the last one.

Requirements in v4.0.3:

• V5.3.3 Verify that context-aware, preferably automated - or at worst, manual - output escaping protects against reflected, stored, and DOM based XSS.
• V5.3.6 Verify that the application protects against JSON injection attacks, JSON eval attacks, and JavaScript expression evaluation.

Requirements at the moment:

• V5.3.3 Verify that context-aware, preferably automated - or at worst, manual - output escaping protects against reflected, stored, and DOM based XSS.
• V5.3.6 Verify that the application protects against JSON injection attacks.

Current V5.3.3 is talking about outout encoding and escaping, and XSS. By this wording it does not add much to the 5.3.1, just the word escaping and this is "a hint" to set the focus for JavaScript

• context-aware - if we limit the requirement to JavaScript, we can skip it
• "preferably automated - or at worst, manual" - what is the "manual encoding" in a web application? Noise.
• "against reflected, stored, and DOM based XSS" - from encoding does not matter, from where the source data came, output must be done correctly anyway.

In a not clearly communicated way we have here 2 requirements:

• when building JavaScript dynamically (including JSON), then escaping and encoding must be in place
• "context-aware" + "against DOM XSS" means we need to use correct methods to use (e. g. display) the input data

The second part was also proposed here #1195 (comment)):

V5.3.3 Verify that safe functions are used to write potentially dynamic content to a web page to avoid the risk of Cross-site Scripting (XSS) vulnerabilities.

And my comment was, that it is a separate problem: #1195 (comment)

Going through all that, I can say that my previous proposal (#1195 (comment) + #1195 (comment)) is still valid.

---

Proposal 1 - remove JavaScript from 5.3.1, as it is covered new 5.3.3:

V5.3.1 Verify that output encoding for an HTTP response, HTML document, or XML document is relevant for the context required, such as encoding the relevant characters for HTML elements, HTML attributes, HTML comments~~, JavaScript~~~, CSS, or HTTP headers, to avoid changing the message or document structure.

Although it is not wrong if it stays there, this overlap may cause confusion.

---

Proposal 2 - make 5.3.3 to concenentrate to JavaScript syntax, including JSON (merge 5.3.6).

Wordsmithing needed, but idea is this:

V5.3.3 Verify that output encoding or escaping is used when presenting data in JavaScript, including JSON, to avoid changing the message or document structure (to avoid JavaScript and JSON injection).

remove current 5.3.6 as merged to 5.3.3.

---

Proposal 3 - a separate requirement to "V50.5 Unintended content interpretation"

One may say, that if you apply user-controlled data with element.innerHTML=input;, then you need to HTML-encode the input to avoid input being executed as HTML. But it is an incorrect solution, as the clear intent was to display the input as plain text and it means that such a solution to display the data must be used.

Parallel example - when an API presents JSON content, but with the header content-type: text/html then the browser is commanded to execute the content. The solution here is not to convert the JSON output to be HTML-displaying friendly but to use the correct content-type - it means the correct way to display it.

The requirement addresses "display method to use from JavaScript or from your UI framework".

Again, wordsmithing needed but I share the idea:

V50.5.* Verify that context-aware methods are used when handling the untrusted data to avoid unintended content execution, such as executing content as HTML instead of displaying it as text.

---

I hope this time it was a clear enough explanation. Also, "content injection" and "content execution" are different things that require different defenses.

[tghosth]

Let's talk through on Thursday

[tghosth]

Proposed requirements:

V5.3.1 [MODIFIED] Verify that output encoding for an HTTP response, HTML document, or XML document is relevant for the context required, such as encoding the relevant characters for HTML elements, HTML attributes, HTML comments, CSS, or HTTP headers, to avoid changing the message or document structure.

V5.3.3 [MODIFIED, SPLIT TO 50.5.*] Verify that output encoding or escaping is used when dynamically building JavaScript content (including JSON), to avoid changing the message or document structure (to avoid JavaScript and JSON injection).

V5.3.6 [DELETED, DUPLICATE of 5.3.3]

V50.5.* [ADDED, SPLIT FROM 5.3.3] Verify that context-aware methods are used when handling untrusted data to avoid unintended content execution, such as executing content as HTML instead of displaying it as text.

@elarlang to open PR