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

fix(scan): first endpoint detection for IP #251

Merged
merged 24 commits into from
Feb 12, 2025

Conversation

psyray
Copy link
Contributor

@psyray psyray commented Jan 17, 2025

Fix #250
Fix #217

Fix first endpoint detection by IPs

  • Scan by hostname
  • Scan by IP

Summary by Sourcery

Improve handling of IP address scans by limiting the available tasks and prioritizing the creation of endpoints for the IP itself, even when rDNS is available.

New Features:

  • Display the number of subdomains associated with each IP and port in their respective badges.

Bug Fixes:

  • Fix an issue where IP addresses were not being properly scanned.

Enhancements:

  • Limit the tasks performed during an IP address scan to only essential ones.
  • Prioritize creating an endpoint for the IP address itself during scans.
  • Improve port information gathering during scans by including service details and uncommon web ports.
  • Update port service information with nmap service detection results.
  • Standardize port information by ensuring only one entry exists per port number.

- Added support for explicitly setting HTTP status during endpoint creation
- Improved error handling in endpoint creation from Nmap data
- Added fallback mechanisms for creating endpoints when initial crawling fails
- Enhanced logging for endpoint and subdomain metadata creation
- Added a safer check for domain existence in nmap data
- Added a check to ensure the scheme is present before creating a fallback endpoint
- Prevents potential errors when creating endpoints from incomplete nmap data
This change enhances the scanner's ability to handle IP addresses as scan targets. Specifically, it limits the tasks performed when scanning an IP, ensures IP addresses are correctly handled in various parts of the code. It also adds the HTTP status code to the endpoint data.
@psyray psyray self-assigned this Jan 17, 2025
Copy link
Contributor

sourcery-ai bot commented Jan 17, 2025

Reviewer's Guide by Sourcery

This pull request enhances the handling of IP address scans by limiting the tasks and correctly creating the first endpoint.

Sequence diagram for IP scan endpoint detection

sequenceDiagram
    participant Scanner
    participant TaskManager
    participant EndpointManager
    participant Database

    Scanner->>TaskManager: initiate_scan(domain)
    TaskManager->>TaskManager: Check if IP scan
    alt is IP scan
        TaskManager->>TaskManager: Filter allowed tasks
        Note over TaskManager: Only allow: port_scan, fetch_url,<br/>dir_file_fuzz, vulnerability_scan,<br/>screenshot, waf_detection
    end

    TaskManager->>EndpointManager: create_first_endpoint_from_nmap_data()
    alt is IP scan
        EndpointManager->>EndpointManager: Create IP endpoint
        EndpointManager->>Database: Save IP endpoint
        opt rDNS hostname exists
            EndpointManager->>EndpointManager: Create rDNS endpoint
            EndpointManager->>Database: Save rDNS endpoint
        end
    else is domain scan
        EndpointManager->>EndpointManager: Create domain endpoint
        EndpointManager->>Database: Save domain endpoint
    end
Loading

Class diagram for endpoint and domain relationships

classDiagram
    class EndPoint {
        +scan_history: ScanHistory
        +target_domain: Domain
        +http_url: String
        +is_default: Boolean
        +discovered_date: DateTime
        +http_status: Integer
    }

    class Domain {
        +name: String
        +last_scan_date: DateTime
    }

    class Subdomain {
        +scan_history: ScanHistory
        +target_domain: Domain
        +name: String
        +discovered_date: DateTime
    }

    Domain "1" -- "*" EndPoint
    Domain "1" -- "*" Subdomain
    Subdomain "1" -- "*" EndPoint

    note for EndPoint "Added http_status field"
    note for Domain "Enhanced IP validation"
Loading

Flow diagram for endpoint creation logic

flowchart TD
    A[Start Endpoint Creation] --> B{Is IP Scan?}
    B -->|Yes| C[Filter Allowed Tasks]
    B -->|No| D[Regular Domain Tasks]

    C --> E{Process Hosts Data}
    E -->|IP Address| F[Create IP Endpoint]
    E -->|rDNS Exists| G[Create rDNS Endpoint]

    D --> H[Create Domain Endpoint]
    H --> I[Try Crawling]
    I -->|Success| J[Save with Crawl Data]
    I -->|Failure| K[Save without Crawling]

    F --> L[Save Metadata]
    G --> L
    J --> L
    K --> L
    L --> M[End]
Loading

File-Level Changes

Change Details Files
Limit tasks for IP scans
  • Added a check to identify if the scan target is an IP address.
  • Filtered the list of tasks to be executed for IP scans, allowing only 'port_scan', 'fetch_url', 'dir_file_fuzz', 'vulnerability_scan', 'screenshot', and 'waf_detection'.
web/reNgine/tasks.py
Improve endpoint creation for IP scans
  • Modified the save_endpoint function to handle IP scans correctly, skipping domain validation for IP addresses.
  • Added http_status parameter to save_endpoint function.
  • Modified create_first_endpoint_from_nmap_data to create endpoints for both the IP and rDNS hostnames when scanning an IP address.
  • Added logic to create an endpoint for the IP itself if it's not present in the Nmap data, using rDNS data if available.
  • Modified save_subdomain to validate both domain and IP formats.
  • Modified save_subdomain to not validate subdomain against domain for IP scans.
web/reNgine/tasks.py

Assessment against linked issues

Issue Objective Addressed Explanation
#250 First endpoint should be correctly detected for hostname or IP
#250 Fix scan by IP not working well

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@psyray psyray linked an issue Jan 17, 2025 that may be closed by this pull request
3 tasks
- This change improves the accuracy and detail of port scanning and service detection during vulnerability scans.
- It expands the range of ports scanned, leverages nmap's service detection capabilities, and stores more detailed service information in the database.
- Additionally, it refactors the handling of scan results to improve efficiency and code clarity.
- It also adds functionality to retrieve ports associated with a subdomain.
This change improves the handling of port information by storing service details specific to each IP address. Previously, port service information was global, which was inaccurate for cases where different IPs on the same domain expose different services on the same port. Now, each IP can have its own service information for a given port. This also affects how port information is displayed in the subdomain view.
This update includes database migrations, API and serializer updates, and adjustments to related functionalities.
This commit refactors the port handling logic and its visualization within the application. The changes improve the backend data model for ports, optimize how port information is fetched and displayed, and enhance the user interface for viewing port details. Specifically, the way ports are associated with IP addresses is improved, and the frontend now dynamically counts and displays the number of IPs associated with each unique port. Additionally, the frontend code for handling port display is streamlined and made more robust.
This change enhances the scan initiation process by correctly handling IP-based scans and improving the parsing of Nmap results. It also adds support for filtering URLs during the scan and fixes a bug where scans would fail if the target host didn't have common web ports open. The update ensures that valid endpoints are created even when dealing with uncommon ports and addresses edge cases in Nmap's output parsing to prevent failures due to unexpected data formats.
This change improves the discovery of web endpoints by expanding the range of ports scanned and refining the endpoint creation logic. The update includes a broader list of common and uncommon HTTP ports, handles various schemes (HTTP and HTTPS) more effectively, and streamlines endpoint creation for both IP addresses and domain names. It also removes an unused URL path filter.
This change simplifies the data model by removing the PortInfo model and establishing a direct many-to-many relationship between Port and IpAddress. Service information, previously stored in PortInfo, is now directly associated with the Port model. This simplification reduces database complexity and improves query efficiency. Additionally, the nmap task signature and command building have been updated to use args instead of cmd for better clarity and flexibility. The report generation logic has also been updated to reflect these changes. Finally, a check for current_subdomain has been added to prevent errors when no subdomain is found.
This change fixes WAF detection by switching to JSON output from wafw00f and fixing the display of WAF information in the UI. The update also fixes a bug where multiple WAFs were not being correctly handled, ensuring only the detected WAF is associated with a subdomain.
- Add new GetIpDetails API view for consolidated IP information
- Update templates to use new endpoint
- Refactor port_display.js to handle new API response format
- Remove redundant API calls and simplify data flow
- Improve error handling and loading states
@AnonymousWP
Copy link
Member

Please solve all comments, then I'll review.

@psyray
Copy link
Contributor Author

psyray commented Feb 9, 2025

Please solve all comments, then I'll review.

Which ones ?
They are all solved for me

@AnonymousWP
Copy link
Member

Please solve all comments, then I'll review.

Which ones ? They are all solved for me

Hmm, on phone it said they weren't resolved, perhaps a bug. On PC it does indeed show they're solved. Will review later.

- Add HTTP/HTTPS links for web ports (common and uncommon)
- Refactor modal display code for better maintainability
- Add subdomain names to IP tooltips
- Improve table layouts with consistent columns
- Add port-specific links for subdomains
- Create reusable modal and table creation functions
- Add UncommonWebPortsView API endpoint
- Update IP serializer to include subdomain names and filtered counts

This commit enhances the user experience by providing direct links to web services
and improves code maintainability through shared components.
- Fix incorrect IP address storage where hostname was stored instead of IP
- Parse IP address directly from nmap XML results
- Add warning log when no IP address is found
- Prioritize IPv4 over IPv6 addresses

This fixes a bug where domain names were being stored in the IpAddress table
instead of actual IP addresses.
- Fix JavaScript syntax error "Uncaught SyntaxError: expected expression, got ','"
- Clean up URL parameters formatting
- Ensure proper comma usage in function parameters
Always use the -Pn flag with nmap to treat all hosts as online, skipping host discovery.
Copy link
Contributor

@0b3ud 0b3ud left a comment

Choose a reason for hiding this comment

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

I have finished testing and reviewing this PR and it works as expected
No bugs detected

@0b3ud 0b3ud merged commit 900b2d6 into release/2.1.1 Feb 12, 2025
5 checks passed
@0b3ud 0b3ud deleted the fix/first-endpoint-detection branch February 12, 2025 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants