-
Notifications
You must be signed in to change notification settings - Fork 14.9k
Align UDP #recvfrom usage with the stdlib #21562
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
base: master
Are you sure you want to change the base?
Changes from all commits
b15f95e
b81af2e
70978a6
d64483f
e707971
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,13 +29,16 @@ def cls | |
|
|
||
| module SocketInterface | ||
| include Rex::Post::Meterpreter::SocketAbstraction::SocketInterface | ||
|
|
||
| MAX_SOCKADDR_LENGTH = 128 | ||
|
|
||
| def type? | ||
| 'udp' | ||
| end | ||
|
|
||
| def recvfrom_nonblock(length, flags = 0) | ||
| data = super(length, flags)[0] | ||
| sockaddr = super(length, flags)[0] | ||
| sockaddr = super(MAX_SOCKADDR_LENGTH, flags)[0] | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This fixes a different bug. What was going on here is that the socket address was being read from the peer (the other end of the socket pair), and the length argument was used for it, which caused a failure when the length wasn't sufficient for the address. The second read should always be for enough data to get the whole address. |
||
| [data, sockaddr] | ||
| end | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -341,7 +341,7 @@ def send_udp(packet,packet_data, nameservers) | |
| @logger.info "Contacting nameserver #{ns} port #{@config[:port]}" | ||
| #socket.sendto(packet_data, ns.to_s, @config[:port].to_i, 0) | ||
| socket.write(packet_data) | ||
| ans = socket.recvfrom(@config[:packet_size]) | ||
| ans = socket.timed_recvfrom(@config[:packet_size]) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the context behind changing this to a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The old Rex::Socket version of |
||
| break if ans | ||
| rescue Timeout::Error | ||
| @logger.warn "Nameserver #{ns} not responding within UDP timeout, trying next one" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -136,7 +136,7 @@ def check | |
| connect_udp | ||
|
|
||
| udp_sock.put(enip_list_identify_pkt) | ||
| res = udp_sock.recvfrom(90) | ||
| res = udp_sock.timed_recvfrom(90) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to other comments, why did this become
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's the same answer. In summary the old |
||
|
|
||
| disconnect_udp | ||
|
|
||
|
|
||
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.
This seems to have removed the
timeouthere, compared to the file below, whereresp, _saddr, _sport = ping_sock.recvfrom(65535, timeout)was changed toresp, = ping_sock.timed_recvfrom(65535, timeout). Why was the timeout removed here?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.
That's right, because it's not necessary here. The 3 lines preceeding this one ensure that the socket is already in a readable state. Then the length field is a maximum length, not a total length, so it'll return immediately even if there is less data than 65535 bytes. Between these two controls, the
#recvfromcall here should not be blocking and it's not necessary to apply a timeout to it.