ASVS
ASVS copied to clipboard
There is no item to check Zip Slip related vulnerabilities
There was a situation that my target was not vulnerable against Directory Traversal Attack in a normal way, but it was allowing me to upload a compressed folder, I was able to upload a PHP shell file in a folder that had the permission to execute the file by doing this: 1- Create a zip file with a Python code and for the name of the target file to be compressed use something like this ../../../../../../../../var/www/target/root/shell.php [we cannot do it using normal applications that can ZIP files] I believe we need to have an ASVS item to check that.
Abstractly, for my taste it's covered by path traversal points. See:
- V12.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.
If the file was written to given destination by zip library, then it's path traversal problem in zip library - should we get zip library specific requirement, I'm not sure. Taken in account, it was "out of box" in 2018, and now most widespread libraries should be fixed or "known to be vulnerable" == avoidable.
Perspective is - that zip library is one of the components in the application, and it should not contain known vulnerabilities:
- V1.14.3 Verify that the build pipeline warns of out-of-date or insecure components and takes appropriate actions.
- V14.2.1 Verify that all components are up to date, preferably using a dependency checker during build or compile time.
I still believe that we need to mention Zip Slip attack as many pentesters might not be aware of this method, maybe we can change 12.1.2 and a note to check Zip Slip attack too.
Extra awareness is never bad, but I think to cover all possible test-cases is more testing-guide context.
At the moment I would say - problem in the issue title is covered, there are ASVS requirements for reporting it.
Let's wait feedback from others as well.
@jmanico What do you think about this one?
I think there is a high likelihood that someone could write this into their own code and therefore just talking about vulnerable dependencies is not enough.
I agree that this is a kind of path traversal vulnerability but (as discussed in #1136) I am not sure if the 12.3 requirements are clear enough to cover this situation as well.
@ImanSharaf please could you look at the requirements in that section and see whether you think they cover the root cause of this vulnerability?
I think there is a high likelihood that someone could write this into their own code
High likelihood, that people implement zip libraries not using zip libraries? If in general "take file path from some container and use it later" then it's not zip specific and should be handled as path traversal.
Mentioned 12.3.1 needs some update anyway.
I agree 100% that zip slip should somehow be mentioned. Two things can go wrong. The library may be totally vulnerable and the developer may not be checking for this path traversal. For example, I typically unzip uploaded files into a staging directiony with OS-level limits....
I agree 100% that zip slip should somehow be mentioned. Two things can go wrong.
Jim, I addressed those both cases in my comment https://github.com/OWASP/ASVS/issues/1222#issuecomment-1036519550
The library may be totally vulnerable ...
If the library is vulnerable, then it is covered by requirements:
- V1.14.3 Verify that the build pipeline warns of out-of-date or insecure components and takes appropriate actions.
- V14.2.1 Verify that all components are up to date, preferably using a dependency checker during build or compile time.
and the developer may not be checking for this path traversal.
Then it's clear path traversal, covered by requirement:
- V12.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.
At the moment I can not see, how this new requirement could be unique - without being duplicate one of mentioned requirements.
So having looked at 12.3, I think we could consider a specific requirement for zipslip or at least a mention of zipslip within an existing item in this section as it already has multiple requirements related to relying on user provided file paths.
To be honest, I think 12.3.1, 12.3.2, 12.3.3 and 12.3.5 all need clarifications to make the differences between them clearer and zipslip could be a part of this.
@elarlang what do you think?
Maybe an addition like so: V12.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. Path traversal can also be exploited by extracting files from a zip file or other archive.
So I disagree with combining it because whilst this is conceptually the same issue, the first part is solved using some sort of URL API but the zip slip thing would need to be fixed in the code that processes the zip files.
So having looked at 12.3, I think we could consider a specific requirement for zipslip or at least a mention of zipslip within an existing item in this section as it already has multiple requirements related to relying on user provided file paths.
ok, let's go for separate requirement.
To be honest, I think 12.3.1, 12.3.2, 12.3.3 and 12.3.5 all need clarifications to make the differences between them clearer and zipslip could be a part of this.
there is separate issue (now) for those requirements: https://github.com/OWASP/ASVS/issues/1427
So how about:
Verify that server-side file processing such as file decompression ignores user provided path information.
If I could not know from this issue that proposed requirement is about zip slip, is it intuitive and understandable, that it is about zip slip? I think it makes sense to mention it somehow in the requirement.
So how about:
Verify that server-side file processing such as file decompression ignores user provided path information to prevent vulnerabilities such as zip slip.
@elarlang
- Level: 1
- CWE-23
Not sure about category, but let's get it into 12.3 and move later to better one if we('ll) have some better one.