Rename 12.2 "File Integrity" to "File Integrity and Content"

[tghosth]

I think 12.1 File Upload should focus on the mechanics around file upload and 12.2 File Integrity should be renamed to "File Integrity and Content" and focus on file type related requirements.

12.1.2 and 12.1.4 should then be moved to 12.2.

#	Description	L1	L2	L3	CWE
12.1.2	Verify that the application checks compressed files (e.g. zip, gz, docx, odt) against maximum allowed uncompressed size and against maximum number of files before uncompressing the file.		✓	✓	409
12.1.4	[ADDED] Verify that the application does not allow uploading compressed files containing symlinks unless this is specifically required (in which case it will be necessary to enforce an allow list of the files that can be symlinked to).		✓	✓	61

This is following a discussion here: https://github.com/OWASP/ASVS/issues/1740#issuecomment-1775145602

@elarlang do you agree?

[elarlang]

I tried to remember the logic how to put requirement to different subcategories. We can put them all together, but maybe there is better way and before we do those changes, let's look the options.

Simplified file upload process.

First step - file upload, file reached to the server and now the actions before going to check file actual content

• maximum allowed size should be checked (12.1.1)
• is it known virus (12.4.2) - kept separately from file upload, as you actually need to check viruses also when serving the file
• archive bomb (12.1.2) - you need to check "is it acrhive" and if it is, what is packing ratio. It is before content check, a lot of files are archives, like xlsx, odt.

2nd step - Is it expected file by content (format)?

• is this file expected and allowed by business logic rules (12.2.1)

3rd step - now, file is expected type and content - do it contain something malicious + can server handle it?

• one can argue - that 12.1.2 belongs here
• 12.1.4 - we need to check inside archive file, is there symlink
• proposed pixel bomb - we need to check image dimensions

Now the question is - do we put them all together or is there good and easy to understand naming to keep them separately?

[tghosth]

Overall, I think it is easier to split into sections wherever possible. (Please remember to use the Chapter > Section > Requirement terminology, I know I am terrible at remembering this as well 🙃)

I would separate them as follows:

File upload process:

file reached to the server and now the actions before going to check file actual content

• maximum allowed size should be checked (12.1.1)

File Integrity:

Is it expected file by content (format)?

• is this file expected and allowed by business logic rules (12.2.1)

File Content:

3rd step - now, file is expected type and content - do it contain something malicious + can server handle it?

• is it known virus (12.4.2) - kept separately from file upload, as you actually need to check viruses also when serving the file
• archive bomb (12.1.2) - you need to check "is it acrhive" and if it is, what is packing ratio. It is before content check, a lot of files are archives, like xlsx, odt.
• 12.1.4 - we need to check inside archive file, is there symlink
• proposed pixel bomb - we need to check image dimensions

We could make these 3 sections or maybe we should combine the first two into 1 section as they both relate to general expectations about uploaded files as opposed to malicious content

[elarlang]

Let's put "V12.3 File execution" in to the game. Entire this section needs work (https://github.com/OWASP/ASVS/issues/1427), but there we have requirement, like zip slip (12.3.7).

If we protect the application, then pixel flood requirement belongs here.

So the outcome from this issue should be - which sections we have and what are the criterias for requirements to belong to some section.

[tghosth]

Hmm, @elarlang do you think we have enough different V12 issues that we should defer to the rework stage?

[elarlang]

I think I can not understand the question.

First priority should be to get requirements in, then we can see what are the needs or possibility for making sections out of those.

[tghosth]

I mean, is this issue now affecting pretty much the whole chapter and therefore best left to when we are working on the whole chapter together at the rework stage?

[elarlang]

Yes, this issue is not priority and is logical outcome when we get more requirements in.

First priority should be to get requirements in, then we can see what are the needs or possibility for making sections out of those.

... and I start hating the phrase "rework stage" already :)

[tghosth]

... and I start hating the phrase "rework stage" already :)

I hear that but I think it helps us come to some sort of conclusion on issues and set ourselves up for the actual rewrite.

[jmanico]

This comment thread is getting a little wild.

I agree that we should change 12.2 from "File Integrity" to "File Integrity and Content" and I'd like to close this out.