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

Make --top-ports and -p a union instead of an intersection #504

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Make --top-ports and -p a union instead of an intersection #504

wants to merge 6 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Aug 13, 2016

This is related to issue #447 and is intended to fix it. There are still issues with the protocol modifiers but only for the --top-ports option. You can find some tests here on the result aiming to show how those two options are mixing up. Note that a scan like ./nmap --top-ports 10 -p 12354,U:20 127.0.0.1 will not scan the UDP ports because you need to specify to Nmap that you want a UDP scan (-sU). You can mix UDP, TCP and SCTP by just adding the corresponding options to enable all the types of scan you want.

Example: Mixing UDP and TCP scans without SCTP would look like this (the verbose version is not available through GitHub, please ask me if needed):
sudo ./nmap -sU -sT --top-ports 3 -p 18,U:11,S:2 127.0.0.1

Resulting in these ports scanned:
Protocol> -p option | --top-ports option
TCP> 18 | 80, 23, 443
UDP> 11, 18 | 631, 161, 137
SCTP> 2

Waiting for feedback!
Cheers

@dmiller-nmap
Copy link

@W0naN0w This seems like it's excessively complicated for what it purports to accomplish. We already have code that parses port specifications, so checking explicitly for [] shouldn't be needed. We should be able to do a set union operation or change the order of how we handle exclusions and inclusions. Here's how it works currently, leaving aside the case of ratio_format == 0, since that always means --top-ports is not usable:

  1. Fill struct scan_lists ptsdata with the ports from -p or all ports ("-").
  2. If --exclude-ports is given, remove those ports from our set.
  3. Loop over services, adding them to a different struct scan_lists called ports if they meet the --top-ports level requirement AND they are in ptsdata.

It's the "AND" there that is the primary problem, resulting in an intersection instead of a union. Instead, I think we can meet our requirement with this algorithm:

  1. If --exclude-ports is given, fill struct scan_lists ptsdata as before, but only use all ports, ignoring -p for now.
  2. Loop over services as before, ending up with the --top-ports set in struct scan_lists ports.
  3. Zero out ptsdata and reinitialize it with -p data.
  4. If --exclude-ports is given, remove those from ptsdata
  5. Use merge_port_lists to combine ports and ptsdata.
  6. Return the merged list.

The parts you would have to figure out are allocation and freeing of the scan_lists structures, conditionals to avoid some parts in the more common cases where only -p or --top-ports are given, and exporting merge_port_lists from nmap.cc so that gettoppts can access it.

A cleaner redesign might use std::set<u16> instead of unordered dynamically-allocated arrays for scan lists, but there's a lot of code that would have to change, and we'd still have to use unordered arrays in the end in order to do port randomization. Don't try to tackle this problem yet.

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.

None yet

1 participant