-
-
Notifications
You must be signed in to change notification settings - Fork 685
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Duplicate 1.12.1 12.4.1 store outside of the web root #1065
Comments
I agree. Can we also clean up 12.4.1 while we are here? |
Agree to remove 1.12.1. 12.4.1 - "preferably with strong validation" does not give anything extra. And even better question is - how it's different from V4 category requirements? In other words - if someone store their files by folder structure in web root, but you can not access them directly with HTTP request, then what is wrong with that solution (argumentation)? |
I also ask, what is the purpose of 12.4.1?
Access control? So attackers cannot access uploaded files?
RCE resistance? So attackers cannot upload executable scripts?
Both?
|
Proposal, as a first step, we can:
Then we can rethink about the point of 12.4.1. |
Good start. I’m happy to take a PR on just that if you like.
|
For the record and for discussion:
For me CWE-522 is better option for current requirement, so I set it for 12.4.1:
PR #1073 |
Current main is now: | 1.12.1 | [DELETED, DUPLICATE TO 12.4.1] | | | | | |
So, duplicate is removed, but I'm not too happy with this requirement. It feels a bit from old-style PHP crappy architecture, where file in public folder leads to RCE. The main question is, should we just blindly disallow to store files in public folder in general, or make it a bit more flexible.
|
I totally agree with you. I think 12.4.1 needs to be re-written with better intent.
|
duplicate resolved, for finetuning I opened separate issue #1087 |
1.12.1 Verify that user-uploaded files are stored outside of the web root.
12.4.1 Verify that files obtained from untrusted sources are stored outside the web root, with limited permissions, preferably with strong validation.
I propose to remove 1.12.1.
The text was updated successfully, but these errors were encountered: