-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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 ntp_nak_to_the_future #19749
Fix ntp_nak_to_the_future #19749
Conversation
f48e4a0
to
092ca5d
Compare
It is no longer in use by any modules. It has been superseded by NTPHeader.
092ca5d
to
5385b3d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix @zeroSteiner! Everything looks good to me. Ran into a small issue setting up the target but have left suggestion fix. Let me know what you think.
Testing
msf6 > use ntp_nak_to_the_future
Matching Modules
================
# Name Disclosure Date Rank Check Description
- ---- --------------- ---- ----- -----------
0 auxiliary/scanner/ntp/ntp_nak_to_the_future . normal No NTP "NAK to the Future"
Interact with a module by name or index. For example info 0, use 0 or use auxiliary/scanner/ntp/ntp_nak_to_the_future
[*] Using auxiliary/scanner/ntp/ntp_nak_to_the_future
msf6 auxiliary(scanner/ntp/ntp_nak_to_the_future) > set rhosts 127.0.0.1
rhosts => 127.0.0.1
msf6 auxiliary(scanner/ntp/ntp_nak_to_the_future) > set rport 123
rport => 123
msf6 auxiliary(scanner/ntp/ntp_nak_to_the_future) > run
[+] 127.0.0.1:123 - NTP - VULNERABLE: Accepted a NTP symmetric active association
[*] Scanned 1 of 1 hosts (100% complete)
[*] Auxiliary module execution completed
documentation/modules/auxiliary/scanner/ntp/ntp_nak_to_the_future.rb.md
Outdated
Show resolved
Hide resolved
5385b3d
to
e5e0657
Compare
Release NotesThere was an issue with the ntp_nak_to_the_future module which was caused by when the BinData::Record instance was sent with the socket, the string representation of it was used instead of the packed binary. It should have been calling #to_binar_s. This fixes the issue with the auxiliary/scanner/ntp/ntp_nak_to_the_future module and documents how the module can be tested. |
This fixes the
auxiliary/scanner/ntp/ntp_nak_to_the_future
module which was broken in commit 5f88971 (landed in #8185). The issue was that when theBinData::Record
instance is sent with the socket, the string representation of it was used instead of the packed binary. It should be calling#to_binar_s
. This PR fixes the issue and documents how the module can be tested.This was the only module that used the
NTPSymmetric
definition and it's been converted to instead use the newNTPHeader
definition which adds more natural handing forNTPTimestamp
andNTPShort
fields. TheNTPHeader
definition was originally written for #19748 and these two PRs share common commits in their history for the new definition.Other NTP modules could be broken and have not been tested. NTPSymmetric had the most overlap with the definition needed for timeroast, so that was the only one that was thoroughly reviewed.
Verification
docker build -t ntpd:4.2.8p3 .
docker run --rm -it --name ntp-server -p 123:123/udp ntpd:4.2.8p3
msfconsole
and use the moduleRHOSTS
value as necessaryDemo