proposal: remove current 5.3.9 as it is in incorrect place and covered by other requirements

[elarlang]

Current requirement 5.3.9

#	Description	L1	L2	L3	CWE
5.3.9	Verify that the application protects against Local File Inclusion (LFI) or Remote File Inclusion (RFI) attacks.	✓	✓	✓	829

CWE-829: Inclusion of Functionality from Untrusted Control Sphere

I try to understand, why requirement 5.3.9 is in category "V5.3 Output Encoding and Injection Prevention" and also, why it exists at all. We may watch it as "path or url injection attack", but defence method here is not encoding or escaping, it must be validation and sanitization.

It's also not clear, is it addressing only PHP style file inclusion which also means that included file is executed as PHP program code. Or is it covering also just file read?

Chosen CWE points more towards V10 category #1471

In "V12.3 File Execution" we have requirements:

#	Description	L1	L2	L3	CWE
12.3.1	Verify that user-submitted filename metadata is not used directly by system or framework filesystems and that a URL API is used to protect against path traversal.	✓	✓	✓	22
12.3.2	Verify that user-submitted filename metadata is validated or ignored to prevent the disclosure, creation, updating or removal of local files (LFI).	✓	✓	✓	73
12.3.3	Verify that user-submitted filename metadata is validated or ignored to prevent the disclosure or execution of remote files via Remote File Inclusion (RFI) or Server-side Request Forgery (SSRF) attacks.	✓	✓	✓	98

• CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')
• CWE-73: External Control of File Name or Path
• CWE-98: Improper Control of Filename for Include/Require Statement in PHP Program ('PHP Remote File Inclusion')

With those requirements we already have "cleanup" issue #1427 as those should belong to input validation and input sanitization categories (but this part will be handled by issue 1427)

As RFI is also always SSRF attack, it's kind of covered by 5.2.6.

#	Description	L1	L2	L3	CWE
5.2.6	Verify that the application protects against SSRF attacks, by validating or sanitizing untrusted data or HTTP file metadata, such as filenames and URL input fields, and uses allow lists of protocols, domains, paths and ports.	✓	✓	✓	918

Reason for Local File Inclusion is most likely some missing validation against allow-listed values and input from user is used to load program code file.

#	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

Proposal: remove current 5.3.9 as it is clearly in incorrect category and it is covered by other requirements (even those othere requirements need to be changed as well)

Label should mark it as DELETED, MERGED TO 12.3.2, 12.3.3 (or where-ever they happen to be via #1427) or if we consider it as "code execution from untrusted sources", then DELETED, MERGED TO 10.3.2 (via #1471)

[tghosth]

Waiting for response on #1427

[tghosth]

History:

#	Description	L1	L2	L3	CWE
5.3.9	Verify that the application protects against Local File Inclusion (LFI) or Remote File Inclusion (RFI) attacks.	✓	✓	✓	829
5.3.4	Verify that file and path handling is not susceptible to Local File Inclusion (LFI) or Remote File Inclusion (RFI) attacks.	✓	✓	✓	3.0
5.7	Verify that the application is not susceptible to Remote File Inclusion (RFI) or Local File Inclusion (LFI) when content is used that is a path to a file.	✓	✓	✓	3.0

I agree that this seems to be covered in section 12 and not convinced there is anything even to merge here... It just seems like a duplicate

[elarlang]

@tghosth - do we wait something for this or it can be done?

[tghosth]

@elarlang let's just eliminate it for now