OWASP / ASVS

Application Security Verification Standard
Creative Commons Attribution Share Alike 4.0 International
2.74k stars 667 forks source link

Consideration for Right-To-Left-Override (RTLO) related vulnerabilities #1744

Closed ImanSharaf closed 2 months ago

ImanSharaf commented 1 year ago

Right-To-Left Override (RTLO) is a unicode control character that is used to reverse the order of characters in languages that are written from right to left. Attackers can use this character to obscure the true extension of a file, potentially tricking users into executing malicious files. For example, an attacker could rename a malicious executable from "evil.exe" to "evilgpj.[RTLO]exe.doc" which would appear as "evilgpjcod.exe" to the user. You can see this write up for Snapchat here.

Verification Requirement:

The application should validate and sanitize input to prevent the misuse of unicode control characters such as RTLO (U+202E).

elarlang commented 1 year ago

On one hand this requires more attention, on the other hand it is covered by current 5.1.4 which requires to validate against allowed characters.

# Description L1 L2 L3 CWE
5.1.3 [MODIFIED] Verify that all input (HTML form fields, REST requests, URL parameters, HTTP headers, cookies, batch files, RSS feeds, etc) is validated using positive validation (allow lists). Where HTML markup must be accepted for input, refer to requirement 5.2.1. (C5) 20
5.1.4 [GRAMMAR] Verify that structured data is strongly typed and validated against a defined schema including allowed characters, length and pattern (e.g. credit card numbers, e-mail addresses, telephone numbers, or validating that two related fields are reasonable, such as checking that suburb and zipcode match). (C5) 20

5.1.4 itself needs some clarification as well, related issue: https://github.com/OWASP/ASVS/issues/1552

tghosth commented 1 year ago

I agree that this should be covered by those requirements.

@elarlang do you think we should close this or do we need to make changes to those requirements to make it clearer?

elarlang commented 1 year ago

I was thinking about it more - I think with current 5.1.3 and 5.1.4 we can catch some cases, but for "plain text input" (like technical issue description field), we may miss this part as "everything is allowed".

I don't clearly propose it (yet), but one way to solve it is actually separate sanitization requirement for that.

But, there is matching requirement as well: # Description L1 L2 L3 CWE
5.2.2 Verify that unstructured data is sanitized to enforce safety measures such as allowed characters and length. 138
ImanSharaf commented 1 year ago

Chat messages are not structured (5.1.4) and also I believe 5.2.2 can be vague.

elarlang commented 1 year ago

If you found the issue, 5.2.2 is suitable for reporting. Just 5.2.2 does not say that you should specially test for that. The main question is, does it require special spotlight or not. If to create separate requirement, I think it sound too much just one edge-case of 5.2.2.

tghosth commented 1 year ago

I think we need to consider this in the overall context of V5 so I think we push this to the "rework" stage.

elarlang commented 1 year ago

Additionally to RTLO, maybe we should in general all non-printable characters here into the discussion?

elarlang commented 1 year ago

If an application needs to accept it, then it must be able to display them correctly as well:

tghosth commented 2 months ago

I think this should be covered by the proposed updates to 5.1.3:

https://github.com/OWASP/ASVS/issues/1640#issuecomment-2283753247

tghosth commented 2 months ago

I think that is covered so I am going to close this,