move V12.3.4 to V12.5 subcategory and rephrase?

[Bankde]

Category / Subcategory
V12.3 File execution Requirements
V12.5 File Download Requirements

Requirement
12.3.4 Verify that the application protects against reflective file download (RFD) by validating or ignoring user-submitted filenames in a JSON, JSONP, or URL parameter, the response Content-Type header should be set to text/plain, and the Content-Disposition header should have a fixed filename.

Recommendation
I have read about RFD here and here. It doesn't look like file execution issue. In JSON exploit case, there isn't even a file at all. It think it's better to move this recommendation to V12.5 which focuses more on downloading file.

Also consider this situation:

1. Uploads malicious malware but names the file as "example.jpg".
2. Upload "example.jpg" into the server.
3. Trick other users to download that "example.jpg" as "example.exe".

I think it's very familiar to the requirement about Content-Type and Content-Disposition header. The current recommendation is a bit too focus on RFD attack so I think we could rephrase to make it more general to cover other cases.
Verify that the application validates or ignoring user-submitted filenames, including in a JSON, JSONP, or URL parameter. The response Content-Type header and Content-Disposition header should be fixed to the file and securely handled by the application instead of the user.

[jmanico]

I suggest avoiding using a user submitted file name (as part of file upload) for storage. I suggest storing the uploaded file using the primary key of the table insert or similar (ir: trusted data) and using the user submitted filename for display purposes only.

…

-- Jim Manico @manicode Secure Coding Education +1 (808) 652-3805

On Mar 12, 2020, at 4:32 AM, Bankde Eakasit ***@***.***> wrote:  Category / Subcategory V12.3 File execution Requirements V12.5 File Download Requirements Requirement 12.3.4 Verify that the application protects against reflective file download (RFD) by validating or ignoring user-submitted filenames in a JSON, JSONP, or URL parameter, the response Content-Type header should be set to text/plain, and the Content-Disposition header should have a fixed filename. Recommendation I have read about RFD here and here. It doesn't look like file execution issue. In JSON exploit case, there isn't even a file at all. It think it's better to move this recommendation to V12.5 which focuses more on downloading file. Also consider this situation: Uploads malicious malware but names the file as "example.jpg". Upload "example.jpg" into the server. Trick other users to download that "example.jpg" as "example.exe". I think it's very familiar to the requirement about Content-Type and Content-Disposition header. The current recommendation is a bit too focus on RFD attack so I think we could rephrase to make it more general to cover other cases. Verify that the application validates or ignoring user-submitted filenames, including in a JSON, JSONP, or URL parameter. The response Content-Type header and Content-Disposition header should be fixed to the file and securely handled by the application instead of the user. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[Bankde]

Hi. I know that's the file name handling best practice and is covered by V12.3.1. I think the current V12.3.4 requirement focuses on verifying that Content-Type and Content-Disposition are not abused or controlled by attacker during download process? That's why I suggest moving to V12.5 File Download Requirements subcategory.

[jamesly123]

I think the fact that RFD allows execution of files on a victim's machine makes it appropriate for this test objective to remain under 'File Execution Requirements'.

[Bankde]

I'm still not yet convinced. Could you share your thought about this requirement too? It also focuses on execution on victim's machine but it's in "File Download Requirements".

12.5.2 Verify that direct requests to uploaded files will never be executed as HTML/JavaScript content.

[elarlang]

My interpretation:

V12.3 File execution Requirements

Problems on the server side with handling upload file name, client/browser is not involved. Malicious code is executed on server side, impact for server

V12.5 File Download Requirements

Problems related to serving files to client. Malicious code is executed on client side, impact for client.

From this perspective I support moving current V12.3.4 to subcategory V12.5 File Download Requirements

V12.3.4 Verify that the application protects against reflective file download (RFD) by validating or ignoring user-submitted filenames in a JSON, JSONP, or URL parameter, the response Content-Type header should be set to text/plain, and the Content-Disposition header should have a fixed filename.

[vanderaj]

This would be a breaking change to numbering and thus is a 4.1 candidate

[jmanico]

We are ready for a PR on this, @elarlang - your move suggestion is spot on.

[Bankde]

Thank you, I will try writing PR this weekend.

[elarlang]

Just in case pointing out (potentially) related opened issues:

#726 - current V12.3.4, proposal:
Verify that the application validates or ignoring user-submitted filenames, including in a JSON, JSONP, or URL parameter. The response Content-Type header and Content-Disposition header should be fixed to the file and securely handled by the application instead of the user.

#721 - current 14.4.2
Verify that all API responses contain Content-Disposition: attachment; filename="api.json" (or other appropriate filename for the content type).

#903 - new, proposal:
Verify that sensitive information is not present in JavaScript or JSONP responses, to prevent cross-origin access to that information.

[elarlang]

Helper for PR: https://github.com/OWASP/ASVS/blob/master/CONTRIBUTING.md

TL;DR:

• For current 12.3.4 mark as description [MOVED TO 12.5.3]
• Before new requirement description (before "Verify that") write label [MODIFIED, MOVED FROM 12.3.4]

[jmanico]

@elarlang - would you like to me to commit the three proposals above or wait for a PR?

[elarlang]

@elarlang - would you like to me to commit the three proposals above or wait for a PR?

just pointing them out for @Bankde to take potentially related things in account (and vice versa). There is no dependency and I think other PR for other issues will be written to other issues. As I have not seen @Bankde being really active here, I just tried to bring some potentially valuable information to avoid "double-work" on that field. No panic :)

[jmanico]

Can I get a PR for 12.3.4 then so we can close this out? Let's do it, there is no need to wait. Let's do it @elarlang !!

[elarlang]

I would like to see other in action as well :) If there is reason why @Bankde don't want or can not do it, I can do it myself, no problem.

[jmanico]

Let's do it!

[Bankde]

@elarlang I really appreciate. However, after looking into detail, these issues look complicated and might require changes on several places. Could @elarlang be the main PR this time? I can still participate by reviewing.

I would say this is unrelated to #903. The sensitive info in Javascript is XSSI. jsonp is also another topic about CSP.

You're right about #721. I will discuss more detail about RFD there.

[elarlang]

I made PR, and also note for myself to recheck this wording when I reach to Content-Type and Content-Disposition headers myself with other issues.

[jmanico]

Thank you for the PR @elarlang :)

[jmanico]

Closed and THANK YOU!