fix: resolve duplicate port & improve first endpoint detection #273
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change improves the port scanning functionality by preventing duplicate port entries and adds cascading deletes for Subdomains, IP addresses, and Ports. It also includes minor updates to logging, datatable display, and app initialization.
This change improves the performance of deleting orphaned ports and adds a database index to the ports ManyToMany field on the IpAddress model. The orphaned port deletion process is now more efficient by using bulk deletion and fetching related objects in bulk. The database index improves query performance for lookups involving IP addresses and their associated ports. A database migration is included to apply the index change.
Improved port management by changing the relationship between IPs and Ports from a ManyToMany field to a ForeignKey on the Port model, directly linking a port to a specific IP. This simplifies database interactions and eliminates redundant data. The logic for handling port creation, updates, and service information association has been centralized and streamlined. Orphaned port and IP cleanup logic, previously handled by signals, has been removed as it's now implicitly managed by the database through cascading deletes.
This change improves port scanning by including all ports for HTTP crawling and removes redundant logic that excluded ports 80 and 443. It also refines the handling of subdomain deletions to ensure orphaned IP addresses are correctly cleaned up after a subdomain is removed, preventing dangling database entries. Additionally, minor logging improvements and code style adjustments are included.
This change improves the handling of imported subdomains by adding automatic Nmap scanning and endpoint creation for each imported subdomain. It also refactors the subdomain deduplication logic to use set comprehension for improved efficiency and readability.
This change refactors the engine task loading mechanism in the subscan modal. It now retrieves tasks based on the selected engine ID instead of the engine name, improving accuracy and efficiency. Additionally, it enhances error handling and user experience by displaying informative messages when no tasks are available or an error occurs during loading. The change also simplifies the task display logic and removes unnecessary code related to port scanning visibility. Finally, it updates the engine ordering to be case-insensitive.
Reviewer's Guide by SourceryThis pull request addresses duplicate port handling, improves first endpoint detection, enhances Nmap result processing, and refactors the UI for better engine task loading. It also includes bug fixes for HTTP crawl and adds signals for cleaning up orphaned IP addresses. Sequence diagram for Port Scanning with get_or_create_portsequenceDiagram
participant PS as Port Scan Task
participant IP as IpAddress
participant P as Port
PS->>IP: Get or create IP Address
IP-->>PS: Returns IP Address
PS->>P: get_or_create_port(ip, port_number)
alt Port exists
P-->>PS: Returns existing Port
else Port does not exist
P->>P: Creates new Port with default values
P-->>PS: Returns new Port
end
PS->>P: Updates Port service information (if available)
P-->>PS: Port updated
Sequence diagram for HTTP Crawl tasksequenceDiagram
participant HT as HTTP Crawl Task
participant S as Subdomain
participant E as Endpoint
HT->>HT: Checks if URLs are provided directly
alt URLs are provided
HT->>HT: Filters URLs based on URL filter
HT->>HT: Writes URLs to input file
else URLs are not provided
HT->>S: Get subdomain endpoints
S-->>HT: Returns HTTP URLs
HT->>HT: Extends URLs with subdomain endpoints
end
HT->>HT: Executes httpx command
loop For each httpx result
HT->>E: Saves/Updates Endpoint
E-->>HT: Returns Endpoint
end
HT->>HT: Removes input file
Sequence diagram for create_first_endpoint_from_nmap_datasequenceDiagram
participant SF as save_imported_subdomains
participant N as Nmap
participant CF as create_first_endpoint_from_nmap_data
participant E as Endpoint
SF->>N: get_nmap_http_datas(subdomain_name, ctx)
N-->>SF: hosts_data
SF->>CF: create_first_endpoint_from_nmap_data(hosts_data, domain, subdomain, ctx)
CF->>E: save_endpoint(http_url, ctx, is_default, subdomain)
E-->>CF: endpoint
CF-->>SF: endpoint
Updated class diagram for Port and IpAddress modelsclassDiagram
class IpAddress {
+id: int
+address: str
+is_cdn: bool
+geo_iso: ForeignKey
+version: int
}
class Port {
+id: int
+number: int
+is_uncommon: bool
+service_name: str
+description: str
+ip_address: ForeignKey
}
Port --|> IpAddress : ForeignKey
note for Port "Unique together constraint on (ip_address, number)"
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Fixes a bug where None values could be written to the HTTP URLs file.
Data migration to correctly update existing port data to use the new ForeignKey field. Some service_name datas will be lost as old system do not track service_name by ip and by port, but only by port
This commit refactors the dashboard charts to enhance data handling and consistency. Key improvements include: More accurate and efficient database queries for chart data. Standardized data retrieval and processing for charts. Improved handling of optional chart data. Corrected a minor typo in a chart title. Added a check for file existence before deletion in http_crawl task. Corrected a redirect issue in admin_interface_update.
3 tasks
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @psyray - I've reviewed your changes - here's some feedback:
Overall Comments:
- The conversion of the javascript charts to use django's templating language is a good change.
- The migration to move the port relationship from a many-to-many to a foreign key looks correct.
Here's what I looked at during the review
- 🟡 General issues: 5 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
I'll review this once suggestions have been implemented and conversations have been resolved. |
|
@psyray I have tested this PR everything works no more port duplication |
This commit introduces significant improvements to the dashboard and backend, focusing on efficient data retrieval and presentation. Key changes include optimized database queries for scan statistics, vulnerability data, and asset information, as well as enhanced timeline visualizations and activity feeds. Additionally, cleanup processes for orphaned data and temporary files have been refined. These enhancements improve performance and provide a more comprehensive overview of project security posture.
Error handling for YAML parsing and subdomain deletion has been enhanced to provide more informative logging and prevent unexpected behavior.
0b3ud
approved these changes
Feb 19, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fix #268
Summary by Sourcery
This pull request addresses several issues related to port scanning, endpoint detection, and data aggregation for the dashboard. It also includes performance improvements, bug fixes, and enhancements to the overall scanning process.
Bug Fixes:
Enhancements:
Build:
Tests:
Chores: