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

Allow Multiple UDP payloads to be specified per port #1859

Closed
wants to merge 3 commits into from

Conversation

@rkirk-r7
Copy link

@rkirk-r7 rkirk-r7 commented Dec 16, 2019

This change allows for multiple UDP payloads to be specified per port. When attempting to retry a payload this will allow for alternative payloads to be attempted.
This also includes additional UDP payloads that rely already included a payload for the same port

  • RMCP ASF
  • DNS-SD additional probes
  • ONCRPC / DCERPC CALL
  • NTP additional probes
  • CIFS NS query probes
  • SNMP additional probes
  • IPSEC / OpenVPN additional probes
  • DNS additional probes
Starting Nmap 7.80SVN ( https://nmap.org ) at 2019-12-13 14:54 GMT
Nmap scan report for ubuntu-1204-64-snmpd.vuln.lax.rapid7.com (10.4.25.141)
Host is up (0.020s latency).

PORT    STATE SERVICE
161/udp open  snmp

Nmap done: 1 IP address (1 host up) scanned in 0.48 seconds

Note: This patch was originally written by Paul Miseiko at Rapid7 (my employer). I'm submitting this upstream with permission and by request. Any errors with the PR are likely mine.

@dmiller-nmap
Copy link

@dmiller-nmap dmiller-nmap commented Dec 30, 2019

Thanks for this contribution! I think this is a great new feature, and I'm on board with most of the patch. Ignore the Travis CI build failure: I made some intervening changes to payload.cc, but I can manually merge this patch around those, so no worries.

I do have one question or concern, though: why does this change from just returning the pointer from std::string::data() to instead using memcpy() to copy into a user-supplied buffer? This seems to me like extra work, adding extra length checks, etc. The docs for data() say that the pointer may be invalidated if modifying methods are called, but once we parse the nmap-payloads file, we don't make changes to the payloads map.

Please let me know the reasoning behind this part of the change so I can get the feature merged. Thanks!

@rkirk-r7
Copy link
Author

@rkirk-r7 rkirk-r7 commented Feb 14, 2020

@dmiller-nmap I'm not sure why it was changed to a memcpy(), I would agree it seems to be an unnecessary step. I have reverted this back to be based on std::string::data() with no ill effects on our scans/usage.

@nmap-bot nmap-bot closed this in 9c83be3 Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.