Skip to content
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

Added Scan Compare feature #877

Merged
merged 14 commits into from
Sep 17, 2024
Merged

Conversation

Captain-T2004
Copy link
Contributor

Checklist

  • I have followed the Contributor Guidelines.
  • The code has been thoroughly tested in my local development environment with flake8 and pylint.
  • The code is Python 3 compatible.
  • The code follows the PEP8 styling guidelines with 4 spaces indentation.
  • This Pull Request relates to only one issue or only one feature
  • I have referenced the corresponding issue number in my commit message
  • I have added the relevant documentation.
  • My branch is up-to-date with the Upstream master branch.

Changes proposed in this pull request

This PR adds a new scan compare feature to Nettacker that aims to make differentiating the results of two scans much easier.

Your development environment

  • OS: Kali GNU/Linux Rolling x86_64
  • OS Version: 6.8.11-amd64
  • Python Version: 3.11.9

nettacker/core/graph.py Fixed Show fixed Hide fixed
nettacker/core/graph.py Fixed Show fixed Hide fixed
nettacker/core/graph.py Fixed Show fixed Hide fixed
nettacker/core/graph.py Fixed Show fixed Hide fixed
Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you take a look at Uncontrolled data used in path expression warnings and also a couple of my comments when you get a chance?

nettacker/api/engine.py Outdated Show resolved Hide resolved
nettacker/core/graph.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consider the suggestions below:

nettacker/api/engine.py Outdated Show resolved Hide resolved
nettacker/core/graph.py Outdated Show resolved Hide resolved
nettacker/core/graph.py Outdated Show resolved Hide resolved
nettacker/core/graph.py Outdated Show resolved Hide resolved
nettacker/core/utils/common.py Outdated Show resolved Hide resolved
@@ -102,6 +102,7 @@ class PathConfig:
tmp_dir = CWD / ".data/tmp"
web_static_dir = PACKAGE_PATH / "web/static"
user_agents_file = PACKAGE_PATH / "lib/payloads/User-Agents/web_browsers_user_agents.txt"
compare_results_base_path = CWD / "results"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Captain-T2004 this is incorrect - compare_results_base_path should be the same as results_dir (see line 101) and as a results I get error
Screenshot 2024-09-13 at 23 16 29

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I will change that back to results_dir, I actually made a separate directory called /results in my Nettacker directory but it was not commited as it was an empty dir. I did this because after adding the path sanitization the user can only save files in the allowed results path with a modifiable file name. This way I though it would be better to find the results fairly easily. I will revert it back but do you think I should alter the path sanitization to include more paths? not just the allowed results_dir path.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Captain-T2004 Please revert back - There is no need to have additional 'results' folder - Nettacker already has one official results folder in the '.data/' for all the results. In order to make comparison results easier you might consider using a file name like this 'results_compare_{scan_id}' - this way it will be easy to find.

@@ -145,6 +146,11 @@ class DefaultSettings(ConfigBase):
usernames_list = None
verbose_event = False
verbose_mode = False
scan_compare_id = None
compare_report_path_filename = "/results_{date_time}_{random_chars}.json".format(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Captain-T2004 shouldn't this be {results_dir} instead of "/results"...?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@securestep9, actually I added the results directory path as a PathConfig object variable. And also here only the name of the file is being generated instead of the complete path. Inside my create_compare_report method in graph.py, I have included the basepath from an PathConfig object and then created a complete path from user input + basepath in PathConfig. This is why I removed {results_dir} from here. The error I was facing was that each time a comparison is ran through API the default file name that was generated here was being rewritten as a new filename was not generated each time. In order to generate a new file name each time I have added some helper functions in /utils/common.py and along with the base filepath from PathConfig I can create a complete path inside my create_compare_report method. If you think this is incorrect do let me know I will reverse it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Captain-T2004 Please use {results_dir} - this is the only folder which shold be for Nettacker results. The problem with file name conflict / files being overwritten can be solved by adding strings to the file name e.g. 'report_compare_{scan_id}' as scan_is is already random it actually makes sense to use scan_id as part of the results filename to identify the results belonging to a specific scan

@@ -117,4 +117,9 @@ username_list: username(s) list, separate with ","
verbose_mode: verbose mode level (0-5) (default 0)
wrong_hardware_usage: "You must select one of these profiles for hardware usage. (low, normal, high, maximum)"
invalid_scan_id: your scan id is not valid!

compare_scans: compare current scan to old scans using the unique scan_id
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Captain-T2004 can you add these messages to another language file e.g. Hindi hi.yaml?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sure

Copy link
Collaborator

@securestep9 securestep9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please check the latest comments

Now the comparison results are stored as report_compare_{date_time}_{scan_id}... in the default results_dir
@Captain-T2004
Copy link
Contributor Author

@securestep9, I have reverted the results back to results_id. Please review.

Signed-off-by: Arkadii Yakovets <[email protected]>
@securestep9 securestep9 self-assigned this Sep 17, 2024
@securestep9 securestep9 added the gsoc GSoC work label Sep 17, 2024
@securestep9 securestep9 merged commit e53fca5 into OWASP:master Sep 17, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gsoc GSoC work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants