refactor: factor out common parts of lfi_detector and sql_injection_detector#2426
refactor: factor out common parts of lfi_detector and sql_injection_detector#2426HarshitPal25 wants to merge 4 commits into
Conversation
|
@kazet here i have centralized the common part of Both lfi_detector and sql_injection_detector, while retaining their specific scanning logic. added a base class for both the file called |
| from artemis.task_utils import get_target_url | ||
|
|
||
|
|
||
| class InjectionDetectorBase(ArtemisBase): |
There was a problem hiding this comment.
I would strongly advise against this approach.
Right now it may work, but then in the future we will most likely add another scanner that has similar approach to lfi etc. and we will need to adjust code just to this.
In artemis you can see that we have a lot of utils-like functions.
Could you please do it this way? While doing so, you can have in mind that reusability not only in the currently extracted two modules.
There was a problem hiding this comment.
Thanks for the feedback! Reworked it to use standalone utility functions in a new injection_utils.py, following the existing utils.py pattern. Extracted
-
strip_query_string— strips query string and fragment from a URL -
is_url_with_parameters— checks if a URL contains query parameters -
create_url_with_batch_payload— builds a URL with multiple params set to a given payload -
collect_urls_to_scan— handles the full pipeline of gathering, filtering, deduplicating, and shuffling URLs before scanning
all plain functions, so any future scanner can reuse them directly without any structural changes.
9414b1a to
34a33ed
Compare
|
@kazet hey bro checkout my work as the review is pending |
|
I think there is more than can be extracted to the common module - LFI detector and SQL injection detector have very similar structure. |
…etector into injection_utils Extract shared utility functions into artemis/injection_utils.py: - strip_query_string: strip query string and fragment from a URL - is_url_with_parameters: check if URL contains query parameters - create_url_with_batch_payload: build URL with batch parameter injection - change_url_params: replace existing URL params and append batch params - collect_urls_to_scan: gather, filter, deduplicate, shuffle URLs for scanning - create_status_reason: build status reason string from scan messages - create_scan_result_data: build standard data dict for scan results Both lfi_detector and sql_injection_detector now import from injection_utils, following the existing *_utils.py pattern for better reusability across future scanner modules. Fixes CERT-Polska#2409
34a33ed to
995cce4
Compare
|
@kazet i have made the changes and change_url_params, create_status_reason and create_scan_result_data is it great if any changes require please let me know |
|
I've missed update on that one, sorry for that. Also as for review; it seems that the logic how scanning actually works can be extracted? |
|
Umm like conflicts were introduced? Can you elaborate it and for scanning logic okey I can extract it too and work on it |
I'm not sure what to elaborate on. Can you ask more specific question? |
|
oh sorry i didn't see the conflicts that were introduced in sql_injection_detector.py i would resolve them and i would extract the scanning. |
Resolved merge conflicts in sql_injection_detector.py by keeping the refactored version that uses utility functions from injection_utils.py. Also fixed self.method references to use imported utility functions.
Extracted the common result processing and saving pattern from both sql_injection_detector and lfi_detector into a reusable helper function in injection_utils.py. This reduces code duplication and centralizes the logic for determining task status and saving scan results.
The create_url_with_batch_payload function was moved to injection_utils, so the test now patches the module-level import in lfi_detector.
The helper function now handles status determination, status_reason creation, and database saving for both injection detectors, reducing code duplication. |
Closes #2409
Summary
Both
lfi_detectorandsql_injection_detectorhad a lot of duplicated structure and logic. This PR introducesInjectionDetectorBase, a shared base class that centralizes the common parts, while each module retains its specific scanning logic.What was factored out into
InjectionDetectorBasenum_retriesSLOW_MODULE_NUM_RETRIES)filters_strip_query_string()is_url_with_parameters()create_url_with_batch_payload()_collect_urls()run())run()How the subclasses work
Each module implements three abstract methods:
scan(urls, task)— module-specific vulnerability scanning logiccreate_status_reason(messages)— formats findings into a status stringcreate_data(messages)— builds the data payload for task resultsLFIDetectoralso overrides_pre_run_check()to callcheck_connection_to_base_url_and_save_error()before scanning.No test changes needed
All existing imports (
LFIDetector,SqlInjectionDetector,LFIFindings,Statements) are still exported from the same modules. The public API, output format, and behavior are fully preserved.Files changed
artemis/modules/injection_detector_base.py— shared base classartemis/modules/lfi_detector.py— now extendsInjectionDetectorBaseartemis/modules/sql_injection_detector.py— now extendsInjectionDetectorBase