5.3.1 has too much going on and should be shortened

[tghosth]

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)

[tghosth]

Point from @ImanSharaf #1561 (comment)

Thank you for your response and for pointing out that ASVS 5.3.1 does technically cover CRLF Injection in HTTP headers. However, I would like to express my concern that the current wording of 5.3.1 might not explicitly highlight the importance of testing for CRLF Injection attacks. As a result, testers might inadvertently overlook this specific attack vector when performing security assessments.

[elarlang]

We also should address CSS related issue with this: #1558

[elarlang]

(updated 2023-03-29, 2023-03-30)

As the Pandora box is opened...

We still need to have abstract requirement to cover all not listed syntaxes:

Verify that output encoding is relevant for the interpreter and context required.

Separate requirements:

• HTML values, HTML attributes
  • can we add here XML values and XML attributes and split up XMl encoding and XPath sanitize from 5.3.10 need more beef #1556
  • the problem here is, "HTML attributes" should be "HTML attribute values". For HTML attributes encoding is not helpful and validation against allow-list and/or sanitation need to be used.
• JavaScript - for JavaScript is available escaping or encoding, both should be covered
  • Maybe we can merge it to current 5.3.3 (need to rethink DOM XSS part from there)
• URL encoding
• HTTP headers
  • Should it be separate requirement or also URL encoding?
  • Should it cover CRLF injection or it should be also separate requirement?
    • there was issue Request for Adding CRLF Injection Check to ASVS #1561
    • and in HTTP/2 context it is covered by current V14.6.3 Verify that a full CRLF (\r\n) sequence is neutralized inside a HTTP/2 header.
  • Should it cover converting cookies (missing at the moment)?
  • Should it cover encoding filename for content-disposition field (proposal/new requirement - served filename in content-disposition header must follow correct encoding #1390)
• SMTP - is there such thing like "SMTP encoding?" or we actually should name it Content-Transfer-Encoding?

I did not cover from 5.3.1 - do we need to mention it?

especially from untrusted inputs (e.g. names with Unicode or apostrophes, such as ねこ or O'Hara).

CSS - (for some reason it is missing from issue description but actually exists in bleeding edge version) - As there is no such thing like CSS encoding, we need to use input validation and/or sanitize when using userinput in CSS. Should be covered via #1558

[jmanico]

Note: CSS encoding is a thing, it's a native JS function. CSS.escape for example.

[tghosth]

I agree that we should split out this requirement as @elarlang has set out above, the treatment should either be one or more of the following:

• Create new requirement if the particular context does not already have a requirement
• Reference this issue and what is needed if there is already a related open issue (such as with HTML values, HTML attributes)
• Make sure that each requirement (existing or new) correctly captures the fix that is needed (encoding, escaping, sanitizing, sandboxing, etc)
• (Don't move existing requirements around yet)
• Make sure that an individual requirement is not mixing different treatments or contexts, e.g. we may want to split out SMTP and IMAP.

[elarlang]

I have started to write those requirements many times.. but those still feel like duplicates. So, there is a turn-around in my proposal, and it is also more aligned with the goals to make fewer requirements to the ASVS and provide "requirement per principle", not "requirement per principle per syntax/technology". We don't have separate requirements for "use authorization correctly in framework x" and "use authorization correctly in framework y", so we also should not have separate requirements for "use encoding correctly for HTML" and "use encoding correctly for URL".

The more I watch the current requirement, the more I think it's ok. Just need to shorten it a bit.

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, and SMTP., and others as the context requires, especially from untrusted inputs (e.g. names with Unicode or apostrophes, such as ねこ or O'Hara). (C4)

The list of examples can be fine-tuned. Splitting the requirement per syntax makes it more like a checklist, and this is a testing guide area.

[elarlang]

For brainstorming - to cover entire chapters 5.2 and 5.3, it's enough to have a requirement:

Verify that the application builds commands and documents without data used and can not change the command and document structure by using relevant defense for the syntax and context required, such as parametrization, encoding, escaping, or sanitization.

Where we can put the line between "too detailed" vs "too abstract".

[jmanico]

CSS - (for some reason it is missing from issue description but actually exists in bleeding edge version) - As there is no such thing like CSS encoding, we need to use input validation and/or sanitize when using userinput in CSS. Should be covered via #1558

CSS encoding does exist in native javascript and other languages meant to escape CSS Variables. See: https://developer.mozilla.org/en-US/docs/Web/API/CSS/escape_static

[elarlang]

Jim, you already said that (#1589 (comment)). Just please, (re)read and learn the issues before commenting :)

[jmanico]

Jim, you already said that (#1589 (comment)). Just please, (re)read and learn the issues before commenting :)

Understood and will do.

[elarlang]

My a bit updated proposal:

Verify that output encoding is relevant for the interpreter and context required, such as encoding value for HTML value, HTML attribute, JavaScript, URL parameter, HTTP header, or SMTP.

[jmanico]

I like it Elar.

[tghosth]

So I am not sure I completely understood your draft, does my rephrasing make sense:

Verify that output encoding is relevant for the interpreter and context required, such as encoding the relevant characters for HTML elements, HTML attribute, JavaScript, URL parameter, HTTP header, or SMTP.

[jmanico]

I’ve been thinking a lot about this and am trying to reduce my concerns to the bare minimum. Base64url encoding is not that important to safe document construction. It’s a little important when adding binary data or JWT’s to a url. But validating untrusted URL’s is crucial to safe document construction. URL’s are the only case where encoding will still lead to client side injection and that is such a common successful attack I want it to be addressed a lot more prominently when it comes to document construction. So my bottom line is: 1) give url validation a very prominent requirement when it comes to document construction. Mentioning url encoding by itself without mentioning validation is dangerous. 2) somehow directly mention base64url encoding when it comes to adding binary data or JWT’s to a url I do not think the current requirements address these prominently enough.

[tghosth]

I think there is still a question here related to document structure which we should discuss further.

Having discussed this further with Elar, the direction becomes a little too general because when you start worrying about building HTML attribute names or element names dynamically or building JSON attribute names dynamically you are getting into the territory of allowing the end user to write their own code as well :) This can't be fixed by encoding so we will leave it for now.

This returns us to how to handle this requirement.

My current feeling is that the fact that the current 5.3.1 requirement is a little too abstract is one of the things that has led to this long discussion.

My current feeling is to have specific requirements which cover the key cases and then a catch-all output encoding for other cases which describes a function's behaviour "takes user input and builds some other representation or command based upon it".

Possible rough splitting for now:

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 attribute, JavaScript, or HTTP headers, to avoid changing the message or document structure.

Verify that the correct encoding is performed for each context when generating a URL.

Verify that the correct encoding is performed for each context in a JSON document.

[jmanico]

Here is a detailed list to consider. Some of these are duplicated from other requirements, admittedly.

HTML Docs

• Verify that user-generated content in HTML tags is properly encoded.
• Verify that user-generated content in HTML attributes is properly encoded.
• Verify that data embedded in JavaScript is encoded using a safe method like JSON encoding or JavaScript-specific encoding functions.
• Verify that data in CSS contexts is encoded to avoid CSS Injection.
• Verify that data within JSON documents is properly encoded to prevent injection attacks.
• Verify that the correct encoding is performed for each context when generating a URL.
• Verify that HTTP headers generated from user input are properly validated and encoded.
• Verify that dynamically generated HTML is constructed safely using templates or frameworks that auto-encode outputs.
• Verify that HTML comments do not contain untrusted user input.

Email

• Verify that email content and headers do not include untrusted user input without proper encoding and validation.

LDAP

• Verify that LDAP queries are built using safe methods that separate code from data.

Database

• Verify that SQL queries use parameterized queries or prepared statements.
• Verify that ORM (Object-Relational Mapping) frameworks are used correctly to prevent SQL Injection.
• Verify that stored procedures are designed to accept parameters that use parameterized queries or prepared statements.

XML

• Verify that XML documents use proper encoding to avoid XML Injection.
• Verify that input used in XML parsing is validated and entity expansion is disabled.

OS

• Verify that command-line arguments are properly escaped or use safe API functions.
• Verify that input used in command execution is properly validated and sanitized.

Logs

• Verify that input used in log messages is properly encoded and sanitized.

[tghosth]

@tghosth to draft some requirements that sit in between 1 requirement for all of the above and individual requirements for everyone one of the above.

[tghosth]

ID	Handled	Category	Requirement	Notes
1	✅	HTML Docs	Verify that user-generated content in HTML tags is properly encoded.	Covered by 5.3.1.
2	✅	HTML Docs	Verify that user-generated content in HTML attributes is properly encoded.	Covered by 5.3.1.
3	✅	HTML Docs	Verify that data embedded in JavaScript is encoded using a safe method like JSON encoding or JavaScript-specific encoding functions.	Jim says below that this is sufficiently covered in 5.3.1.
4	✅	HTML Docs	Verify that data in CSS contexts is encoded to avoid CSS Injection.	Mentioned in both 5.3.1 and 5.2.8. Opened #1994 to discuss/resolve this.
5	✅	HTML Docs	Verify that data within JSON documents is properly encoded to prevent injection attacks.	Mentioned in 5.3.6 and discussed in #1555
6	✅	HTML Docs	Verify that the correct encoding is performed for each context when generating a URL.	Being discussed in #1961
7	✅	HTML Docs	Verify that HTTP headers generated from user input are properly validated and encoded.	Covered by 5.3.1.
8	✅	HTML Docs	Verify that dynamically generated HTML is constructed safely using templates or frameworks that auto-encode outputs.	Jim says below that this is not sufficiently covered in 5.2.5 and "I think safely building HTML (encoding and similar) and avoiding template injection (avoid server side template assembly) are different controls.". Opened #1998 to discuss this separately.
9	✅	HTML Docs	Verify that HTML comments do not contain untrusted user input.	Added to 5.3.1 and tracked by #1997.
10	✅	Email	Verify that email content and headers do not include untrusted user input without proper encoding and validation.	Covered by 5.2.3.
11	✅	LDAP	Verify that LDAP queries are built using safe methods that separate code from data.	Covered by 5.3.7
12	✅	Database	Verify that SQL queries use parameterized queries or prepared statements.	Covered by 5.3.4.
13	✅	Database	Verify that ORM (Object-Relational Mapping) frameworks are used correctly to prevent SQL Injection.	Covered by 5.3.4.
14	✅	Database	Verify that stored procedures are designed to accept parameters that use parameterized queries or prepared statements.	Opened #1993 to include this in 5.3.4.
15	✅	XML	Verify that XML documents use proper encoding to avoid XML Injection.	Covered by 5.3.10.
16	✅	XML	Verify that input used in XML parsing is validated and entity expansion is disabled.	XXE is covered in 5.3.10 and schema validation is in 13.3.1.
17	✅	OS	Verify that command-line arguments are properly escaped or use safe API functions.	Covered in 5.3.8.
18	✅	OS	Verify that input used in command execution is properly validated and sanitized.	Covered in 5.3.8.
19	✅	Logs	Verify that input used in log messages is properly encoded and sanitized.	Covered in 7.3.1.

[jmanico]

"Verify that data embedded in JavaScript is encoded using a safe method like JSON encoding or JavaScript-specific encoding functions." @jmanico do you think this is sufficiently covered in 5.2.5?

Politely, no. I think safely building HTML (encoding and similar) and avoiding template injection (avoid server side template assembly) are different controls.

[jmanico]

"Verify that data embedded in JavaScript is encoded using a safe method like JSON encoding or JavaScript-specific encoding functions." @jmanico do you think this is sufficiently covered in 5.3.1?

Yes, for sure.

[jmanico]

I think we need both 5.2.8 and 5.3.1
5.2.8 | Verify that the application sanitizes, disables, or sandboxes user-supplied scriptable or expression template language content, such as Markdown, CSS or XSL stylesheets, BBCode, or similar.
-- | --

This addresses sanitizing user authored cunks of CSS. Just like untrusted HTML is need to be sanitized.

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

This addresses adding user controlled data, like a color, into blocks of otherwise static css. Different controls and to some degree different attacks, too.

[tghosth]

I updated the table above.

[tghosth]

I merged this in. I added a catch all for any encoding scenarios not covered elsewhere as I suggested here.

I don't think this wording is perfect but it has taken too long to spend further on it for now.

[elarlang]

I merged this in. I added a catch all for any encoding scenarios not covered elsewhere #1589 (comment).

#	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
5.3.14	[ADDED] Verify that output encoding is relevant for the interpreter and context required in any context where a potentially dangerous interpreter, not mentioned above, is being used.		✓	✓

I prefer to not have the 5.3.14 and have 5.3.1 to cover all cases.

The requirement text must be independent and understandable when watched just a requirement alone - it can not contain "not mentioned above".

[tghosth]

I'd really rather keep 5.3.1 focused on it's particular area. Would it be better to just change the text to say:

[ADDED] Verify that output encoding is relevant for the interpreter and context required in any context where a potentially dangerous interpreter , not mentioned above, is being used.

[jmanico]

Looks good @tghosth @elarlang

[tghosth]

Whilst the concept of a catch-all makes sense, having thought about it more, I don't think it adds that much value compared to adding another, potentially confusing requirement.

[tghosth]

We also have 5.2.2 as a sort of catch all for dangerous sinks🙃

[tghosth]

@set-reminder 7 days @tghosth to remove 5.3.14

[octo-reminder]

⏰ Reminder
Thursday, August 22, 2024 12:00 AM (GMT+02:00)

@tghosth to remove 5.3.14

[octo-reminder]

🔔 @tghosth

@tghosth to remove 5.3.14

[tghosth]

5.3.14 should be removed in #2026