-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add tests for Archive Directory Traversal #1138
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Please address the bot's feedback. (Yes we know some of it might be pre-existing.) |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
@kingthorin checks passing now. Let me know if I need to revert any of those zip --> ZIP changes. |
That stuff looks good to me. There are a few parts that read a bit awkwardly but I"ll do a review tomorrow and point them out or suggest alternative wording. |
@kingthorin the text was indeed awkward. I sent a new version trying to bring it closer to the wstg style guide. |
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@kingthorin I sent a new update based on your previous comments. |
Thanks, sorry I wasn't able to tackle it over the weekend. Will try to get it knocked off later today. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a few comments based on my thoughts - I think that it's a good thing test to add, but we don't really give enough details here to allow someone reading the guide to actual test for this.
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Show resolved
Hide resolved
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
…Testing/09-Test_Upload_of_Malicious_Files.md
…Testing/09-Test_Upload_of_Malicious_Files.md
…Testing/09-Test_Upload_of_Malicious_Files.md
This comment was marked as outdated.
This comment was marked as outdated.
…Testing/09-Test_Upload_of_Malicious_Files.md
…Testing/09-Test_Upload_of_Malicious_Files.md
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
…Testing/09-Test_Upload_of_Malicious_Files.md
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slightly late review, let me know if any of my recommendations don't make sense. Hopefully we can merge this afterwards :)
Thank you for your contributions!
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
…Testing/09-Test_Upload_of_Malicious_Files.md Co-authored-by: ThunderSon <[email protected]>
…Testing/09-Test_Upload_of_Malicious_Files.md Co-authored-by: ThunderSon <[email protected]>
…Testing/09-Test_Upload_of_Malicious_Files.md Co-authored-by: ThunderSon <[email protected]>
…Testing/09-Test_Upload_of_Malicious_Files.md Co-authored-by: ThunderSon <[email protected]>
…Testing/09-Test_Upload_of_Malicious_Files.md Co-authored-by: ThunderSon <[email protected]>
…Testing/09-Test_Upload_of_Malicious_Files.md Co-authored-by: ThunderSon <[email protected]>
..._Application_Security_Testing/10-Business_Logic_Testing/09-Test_Upload_of_Malicious_Files.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the work here, this looks good to go!
This PR fixes #845
What did this PR accomplish?
Thank you for your contribution!