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.

Sign up for GitHub

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

Major bug fixes to the capture mixin and nbns_response module #4827

Merged
merged 6 commits into from
Mar 17, 2015
Merged

Major bug fixes to the capture mixin and nbns_response module #4827

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
615d71d
Remove extraneous calls to GC.start()
Feb 23, 2015
bdd5276
This fixes a number of issues with the Capture mixin
Feb 23, 2015
9730a16
Small cleanups to the LLMR responder module
Feb 23, 2015
1b1716b
Fix a handful of bugs that broke this modules. Fixes #4799
Feb 23, 2015
1001061
Initialize @capture_count
Mar 5, 2015
2ea9844
while(true)->loop, use thread.join
Mar 16, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 45 additions & 58 deletions lib/msf/core/exploit/capture.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def initialize(info = {})
[
true,
'Send a TTL=1 random UDP datagram to this host to discover the default gateway\'s MAC',
'www.metasploit.com']),
'8.8.8.8']),
OptPort.new('GATEWAY_PROBE_PORT',
[
false,
Expand Down Expand Up @@ -143,7 +143,6 @@ def close_pcap
return unless self.capture
self.capture = nil
self.arp_capture = nil
GC.start()
end

def capture_extract_ies(raw)
Expand All @@ -163,26 +162,15 @@ def capture_extract_ies(raw)
end

#
# This monstrosity works around a series of bugs in the interrupt
# signal handling of Ruby 1.9
# Loop through each packet
#
def each_packet
return unless capture
begin
@capture_count = 0
reader = framework.threads.spawn("PcapReceiver", false) do
capture.each do |pkt|
yield(pkt)
@capture_count += 1
end
end
reader.join
rescue ::Exception
raise $!
ensure
reader.kill if reader.alive?
@capture_count ||= 0
capture.each do |pkt|
yield(pkt)
@capture_count += 1
Copy link
Contributor

Choose a reason for hiding this comment

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

@capture_count is not initialized at this point, so we're attempting to increment a nil by one.

end

@capture_count
end

Expand Down Expand Up @@ -242,10 +230,9 @@ def inject_pcap(pcap_file, filter=nil, delay = 0, pcap=self.capture)
pcap.inject(pkt)
Rex.sleep((delay * 1.0)/1000)
end
GC.start
end

# Capture_sendto is intended to replace the old Rex::Socket::Ip.sendto method. It requires
# capture_sendto is intended to replace the old Rex::Socket::Ip.sendto method. It requires
# a payload and a destination address. To send to the broadcast address, set bcast
# to true (this will guarantee that packets will be sent even if ARP doesn't work
# out).
Expand All @@ -262,24 +249,20 @@ def capture_sendto(payload="", dhost=nil, bcast=false, dev=nil)

# The return value either be a PacketFu::Packet object, or nil
def inject_reply(proto=:udp, pcap=self.capture)
reply = nil
to = (datastore['TIMEOUT'] || 500).to_f / 1000.0
if not pcap
raise RuntimeError, "Could not access the capture process (remember to open_pcap first!)"
else
begin
::Timeout.timeout(to) do
pcap.each do |r|
packet = PacketFu::Packet.parse(r)
next unless packet.proto.map { |x| x.downcase.to_sym }.include? proto
reply = packet
break
end
# Defaults to ~2 seconds
to = (datastore['TIMEOUT'] * 4) / 1000.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Here and a couple of other places, the handling of TIMEOUT is not consistent. Here, you assume it will be present and > 0, but previously it defaulted to 500 (ms?). Where is TIMEOUT defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Defined in the mixin as 500ms, the previous timeouts were too low in some cases (wifi, vpn pivot, etc).

raise RuntimeError, "Could not access the capture process (remember to open_pcap first!)" if not pcap
begin
::Timeout.timeout(to) do
pcap.each do |r|
packet = PacketFu::Packet.parse(r)
next unless packet.proto.map { |x| x.downcase.to_sym }.include? proto
return packet
end
rescue ::Timeout::Error
end
rescue ::Timeout::Error
end
return reply
nil
end

# This ascertains the correct Ethernet addresses one should use to
Expand Down Expand Up @@ -328,20 +311,19 @@ def probe_gateway(addr)
end

begin
to = (datastore['TIMEOUT'] || 1500).to_f / 1000.0
to = ((datastore['TIMEOUT'] || 500).to_f * 8) / 1000.0
::Timeout.timeout(to) do
while (my_packet = inject_reply(:udp, self.arp_capture))
if my_packet.payload == secret
dst_mac = self.arp_cache[:gateway] = my_packet.eth_daddr
src_mac = self.arp_cache[Rex::Socket.source_address(addr)] = my_packet.eth_saddr
return [dst_mac, src_mac]
else
next
end
loop do
my_packet = inject_reply(:udp, self.arp_capture)
next unless my_packet
next unless my_packet.payload == secret
dst_mac = self.arp_cache[:gateway] = my_packet.eth_daddr
src_mac = self.arp_cache[Rex::Socket.source_address(addr)] = my_packet.eth_saddr
return [dst_mac, src_mac]
end
end
rescue ::Timeout::Error
# Well, that didn't work (this common on networks where there's no gatway, like
# Well, that didn't work (this is common on networks where there's no gateway, like
# VMWare network interfaces. We'll need to use a fake source hardware address.
self.arp_cache[Rex::Socket.source_address(addr)] = "00:00:00:00:00:00"
end
Expand All @@ -354,26 +336,31 @@ def arp(target_ip=nil)
return self.arp_cache[:gateway] unless should_arp? target_ip
source_ip = Rex::Socket.source_address(target_ip)
raise RuntimeError, "Could not access the capture process." unless self.arp_capture

p = arp_packet(target_ip, source_ip)
inject_eth(:eth_type => 0x0806,
:payload => p,
:pcap => self.arp_capture,
:eth_saddr => self.arp_cache[Rex::Socket.source_address(target_ip)]
)
begin
to = (datastore['TIMEOUT'] || 500).to_f / 1000.0
::Timeout.timeout(to) do
while (my_packet = inject_reply(:arp, self.arp_capture))
if my_packet.arp_saddr_ip == target_ip

# Try up to 3 times to get an ARP response
1.upto(3) do
inject_eth(:eth_type => 0x0806,
:payload => p,
:pcap => self.arp_capture,
:eth_saddr => self.arp_cache[Rex::Socket.source_address(target_ip)]
)
begin
to = ((datastore['TIMEOUT'] || 500).to_f * 8) / 1000.0
::Timeout.timeout(to) do
loop do
my_packet = inject_reply(:arp, self.arp_capture)
next unless my_packet
next unless my_packet.arp_saddr_ip == target_ip
self.arp_cache[target_ip] = my_packet.eth_saddr
return self.arp_cache[target_ip]
else
next
end
end
rescue ::Timeout::Error
end
rescue ::Timeout::Error
end
nil
end

# Creates a full ARP packet, mainly for use with inject_eth()
Expand Down
1 change: 0 additions & 1 deletion lib/msf/core/exploit/ipv6.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ def close_icmp_pcap()

return if not @ipv6_icmp6_capture
@ipv6_icmp6_capture = nil
GC.start()
end

#
Expand Down
1 change: 0 additions & 1 deletion modules/auxiliary/spoof/arp/arp_poisoning.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ def run

if datastore['LISTENER']
@listener.kill if @listener
GC.start()
end

if capture and @spoofing and not datastore['BROADCAST']
Expand Down
29 changes: 17 additions & 12 deletions modules/auxiliary/spoof/llmnr/llmnr_response.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,7 @@ def dispatch_request(packet, rhost, src_port)
end
ip_pkt.recalc

open_pcap
capture_sendto(ip_pkt, rhost.to_s, true)
close_pcap
capture_sendto(ip_pkt, rhost.to_s, true)
end

def monitor_socket
Expand Down Expand Up @@ -176,7 +174,10 @@ def should_print_reply?(host)

def run
check_pcaprub_loaded()
::Socket.do_not_reverse_lookup = true
::Socket.do_not_reverse_lookup = true # Mac OS X workaround

# Avoid receiving extraneous traffic on our send socket
open_pcap({'FILTER' => 'ether host f0:f0:f0:f0:f0:f0'})
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this magic MAC?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any MAC that doesn't match normal traffic. The Capture mixin creates two Pcap interfaces, one for ARP and GW discovery, and another for sending responses. Prior to this change, every send would open_pcap, capture_sendto, and close_pcap, which was inefficient. The new code calls open_pcap at the start and close_pcap in cleanup, but that means any traffic hitting the default filter will start to get buffered. The bogus host filter prevents random traffic from queueing up on the send Pcap instance. In short, it is probably more preventative than anything else, but doesn't hurt anything either.

Copy link
Contributor

Choose a reason for hiding this comment

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

This appears to break the ability to use an interface of any:

[-] Auxiliary failed: PCAPRUB::BPFError invalid bpf filter
[-] Call stack:
[-]   lib/msf/core/exploit/capture.rb:142:in `setfilter'
[-]   lib/msf/core/exploit/capture.rb:142:in `open_pcap'
[-]   /home/egypt/repo/metasploit-framework/modules/auxiliary/spoof/nbns/nbns_response.rb:156:in `run'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using any may not work for injection anyways. With the filter removed, are sent packets actually ending up on the right interface?

Copy link
Contributor

Choose a reason for hiding this comment

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

So far I haven't seen injected packets on the correct interface anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mind testing it once the filter is removed from this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Strike that, it's working if i use a real, wired interface. Fails on VMWare ifaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, bizarre. Should we report a warning or something if either any or vmnet.* is specified?


# Multicast Address for LLMNR
multicast_addr = ::IPAddr.new("224.0.0.252")
Expand All @@ -191,24 +192,28 @@ def run
self.sock = Rex::Socket.create_udp(
# This must be INADDR_ANY to receive multicast packets
'LocalHost' => "0.0.0.0",
'LocalPort' => 5355)
'LocalPort' => 5355,
'Context' => { 'Msf' => framework, 'MsfExploit' => self }
)
self.sock.setsockopt(::Socket::SOL_SOCKET, ::Socket::SO_REUSEADDR, 1)
self.sock.setsockopt(::Socket::IPPROTO_IP, ::Socket::IP_ADD_MEMBERSHIP, optval)

self.thread = Rex::ThreadFactory.spawn("LLMNRServerMonitor", false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why not framework's thread factory?

Copy link
Contributor

Choose a reason for hiding this comment

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

Its the same, framework takes over the one in Rex when the higher level threadfactory inits @@Provider.

monitor_socket
monitor_socket
}

print_status("LLMNR Spoofer started. Listening for LLMNR requests with REGEX \"#{datastore['REGEX']}\" ...")

add_socket(self.sock)

while thread.alive?
select(nil, nil, nil, 0.25)
end

self.thread.kill
self.sock.close rescue nil
self.thread.join
end

def cleanup
if self.thread and self.thread.alive?
self.thread.kill
self.thread = nil
end
close_pcap
Copy link
Contributor

Choose a reason for hiding this comment

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

won't this already be called by the capture mixin's cleanup?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only if it called super(), but the Capture mixin doesn't have a cleanup() method, so this was a resource leak.

end
end
Loading