zap-extensions
zap-extensions copied to clipboard
zaproxy
ascanrules: Fix FP in Path Traversal directory browsing checks
Overview
This is part of PR 5336. PathTraversal directory browsing detects (Check 3): This check finds a large number of FP in various web applications, because the regular expression that parses the response from the server for the nix architecture will not accurately determine that this is the root directory of the OS. For example, to generate FP on payload "c:" or any other from the LOCAL_DIR_TARGETS array, the words in the response from the server will be enough (etc. , bootstrap.min.js , tabindex) to generate Aletr. The reason for this is a weak pattern check for nix systems implemented in DirNamesContentsMatcher. You can play this FP on Apache Superset.
In this PR checks for windows and nix directories are implemented with different functions and enhanced regex for nix architecture.
Checklist
- [ ] Update help
- [x] Update changelog
- [x] Run
./gradlew spotlessApplyfor code formatting - [x] Write tests
- [ ] Check code coverage
- [x] Sign-off commits
- [ ] Squash commits
- [x] Use a descriptive title
@kingthorin I figured it out in more detail. There are two scenarios in this situation.
- It can be directory browsing at the web server level. In this case, you can focus on the examples of the response from the server.
1.1 "<td><a href=\"/etc/\">etc</a></td>" ...
1.2 "<li><a href="bin/">bin/</a></li>" ...
- It can be directory browsing at the web application level and be given in plain text.
curl http://localhost:4444/ls\?directory\=../../../
sbin
usr
mnt
lib
...
root
srv
opt
home
etc
bin
For this reason, I decided to replace regex. Slightly increase the number of checks to get rid of FP accurately, as well as remove the hardcoded "href" in order to match plain text.
@kingthorin What should I do to get this PR merged?) How do I invite other Reviewers?
As mentioned in https://github.com/zaproxy/zap-extensions/pull/5336#issuecomment-2114273656 you should rebase both PRs.
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Okay so we have two options:
- You turn on "Allow edits by maintainers" (you should see it in the menu on the right). Then I can push a copy in the proper state.
- You check this comparison: https://github.com/zaproxy/zap-extensions/compare/main...kingthorin:zap-extensions:pathTraversalDirBased?expand=1
- Assuming it's correct you add my fork as a git remote
git remote add kingthorin https://github.com/kingthorin/zap-extensions.git - Fetch my remote
git fetch kingthorin - Delete your local branch
git branch -d pathTraversalDirBased(If it complains that it isn't fully merged you might need to use-D) - Checkout the branch on my fork (that's in the proper state)
git checkout kingthorin/pathTraversalDirBased - Push that back to your fork, replacing the messed up version
git push -u origin pathTraversalDirBased
- Assuming it's correct you add my fork as a git remote
@kingthorin Please tell me if I can try the following:
- Update the fork of the organization;
- Create a new branch;
- Cheery-pick the necessary commits;
- Push a branch into a fork;
- Checkout on pathTraversalDirBased from this PR
- Rebase pathTraversalDirBased from this PR on new_branch
- Push new state of pathTraversalDirBased
Discussed this with the team. We think that "should" work, though it actually seems like more effort than my option 2 😉
@kingthorin I am trying your option 2. But I encounter the following error at the last stage.
git push -u origin pathTraversalDirBased
To https://github.com/vulnspace/zap-extensions.git
! [rejected] pathTraversalDirBased -> pathTraversalDirBased (non-fast-forward)
error: failed to push some refs to 'https://github.com/vulnspace/zap-extensions.git'