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

Refactor definitions #606

Merged

Conversation

devl00p
Copy link
Contributor

@devl00p devl00p commented Jul 14, 2024

This MR refactors the attributes of each definition, which were previously held as module constants, into separate classes named "findings".

Purpose:

  • Enhanced Type Safety: By leveraging type hinting, functions like add_payload can now benefit from stronger data type enforcement.
  • Attribute Completeness: The use of an abstract base class ensures that all required attributes are present for each finding, reducing the risk of omissions (linter will flag any missing attributes).
    Parameter Simplification: Encapsulating attributes within a single class instance reduces the number of parameters - required for functions like add_payload.
  • Fixed Finding Types: Each finding class is assigned a specific type (vulnerability, anomaly, or additional), ensuring clarity and consistency.

The severity of a finding is not encapsulated within the finding class itself, as it may vary depending on specific circumstances.

If a finding can be categorized as both a vulnerability and an additional type, separate finding classes should be created for each category.

@devl00p devl00p force-pushed the move_definitions_to_finding_classes branch 2 times, most recently from 32e62fc to 6d939e3 Compare July 14, 2024 15:24
@fwininger
Copy link
Contributor

@bretfourbe peux tu regarder les implications pour nous.

@devl00p devl00p force-pushed the move_definitions_to_finding_classes branch from cf83504 to a45f81f Compare July 14, 2024 21:16
Copy link

codecov bot commented Jul 14, 2024

Codecov Report

Attention: Patch coverage is 93.23374% with 77 lines in your changes missing coverage. Please review.

Project coverage is 80.40%. Comparing base (7e84c4c) to head (5ca9ce1).
Report is 1 commits behind head on master.

Files Patch % Lines
wapitiCore/definitions/base.py 75.75% 8 Missing ⚠️
wapitiCore/definitions/xpath.py 75.00% 6 Missing ⚠️
wapitiCore/attack/mod_xxe.py 60.00% 4 Missing ⚠️
wapitiCore/definitions/xxe.py 83.33% 4 Missing ⚠️
wapitiCore/attack/mod_sql.py 62.50% 3 Missing ⚠️
wapitiCore/definitions/http_headers.py 95.58% 3 Missing ⚠️
wapitiCore/attack/mod_csp.py 33.33% 2 Missing ⚠️
wapitiCore/attack/mod_exec.py 71.42% 2 Missing ⚠️
wapitiCore/attack/mod_file.py 66.66% 2 Missing ⚠️
wapitiCore/attack/mod_http_headers.py 60.00% 2 Missing ⚠️
... and 39 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #606      +/-   ##
==========================================
+ Coverage   79.61%   80.40%   +0.78%     
==========================================
  Files         123      124       +1     
  Lines       10431    11125     +694     
==========================================
+ Hits         8305     8945     +640     
- Misses       2126     2180      +54     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@devl00p devl00p force-pushed the move_definitions_to_finding_classes branch from 28dcc65 to 40a086f Compare July 15, 2024 16:27
@devl00p
Copy link
Contributor Author

devl00p commented Jul 15, 2024

Another significant change: the HTTP headers definitions are now split into 3 separate findings:

  • Clickjacking (tied to X-Frame-Options)
  • MIME Type Confusion (tied to X-Content-Type-Options)
  • HSTS (did not changed that one but we may consider renaming it to "Man in the middle" or something else)

@devl00p devl00p requested a review from bretfourbe July 16, 2024 10:04
tests/attack/test_mod_htp.py Outdated Show resolved Hide resolved
tests/attack/test_mod_htp.py Outdated Show resolved Hide resolved
tests/attack/test_mod_htp.py Outdated Show resolved Hide resolved
tests/attack/test_mod_log4shell.py Outdated Show resolved Hide resolved
tests/attack/test_mod_log4shell.py Outdated Show resolved Hide resolved
wapitiCore/attack/mod_upload.py Outdated Show resolved Hide resolved
wapitiCore/controller/wapiti.py Show resolved Hide resolved
wapitiCore/definitions/backup.py Show resolved Hide resolved
wapitiCore/parsers/commandline.py Show resolved Hide resolved
tests/integration/wapiti/templates_and_data.py Outdated Show resolved Hide resolved
@devl00p devl00p requested a review from bretfourbe July 18, 2024 07:28
@devl00p devl00p merged commit 20bcd67 into wapiti-scanner:master Jul 19, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants