From 505d07578f7eb38bf9dd7a1044ae9d6574c80d18 Mon Sep 17 00:00:00 2001
From: adfoster-r7 <alandavid_foster@rapid7.com>
Date: Wed, 13 Sep 2023 10:03:15 +0100
Subject: [PATCH] Fix hanging forever if rsock doesnt connect

---
 .github/workflows/verify.yml | 17 ++++------
 lib/rex/socket.rb            | 62 ++++++++++++++++++++++++------------
 spec/rex/socket_spec.rb      | 38 ++++++++++++++++++++++
 3 files changed, 86 insertions(+), 31 deletions(-)

diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml
index dded6cc..b40ed08 100644
--- a/.github/workflows/verify.yml
+++ b/.github/workflows/verify.yml
@@ -30,7 +30,7 @@ jobs:
     timeout-minutes: 40
 
     strategy:
-      fail-fast: true
+      fail-fast: false
       matrix:
         ruby:
           - '2.7'
@@ -39,20 +39,20 @@ jobs:
           - '3.2'
         os:
           - ubuntu-20.04
+          - windows-2019
+          - macos-11
           - ubuntu-latest
         exclude:
           - { os: ubuntu-latest, ruby: '2.7' }
           - { os: ubuntu-latest, ruby: '3.0' }
-        test_cmd:
-          - bundle exec rspec
 
     env:
       RAILS_ENV: test
 
-    name: ${{ matrix.os }} - Ruby ${{ matrix.ruby }} - ${{ matrix.test_cmd }}
+    name: ${{ matrix.os }} - Ruby ${{ matrix.ruby }}
     steps:
       - name: Checkout code
-        uses: actions/checkout@v2
+        uses: actions/checkout@v4
 
       - name: Setup Ruby
         uses: ruby/setup-ruby@v1
@@ -60,9 +60,6 @@ jobs:
           ruby-version: ${{ matrix.ruby }}
           bundler-cache: true
 
-      - name: ${{ matrix.test_cmd }}
+      - name: rspec
         run: |
-          echo "${CMD}"
-          bash -c "${CMD}"
-        env:
-          CMD: ${{ matrix.test_cmd }}
+          bundle exec rspec
diff --git a/lib/rex/socket.rb b/lib/rex/socket.rb
index f58b28e..c5a23a1 100644
--- a/lib/rex/socket.rb
+++ b/lib/rex/socket.rb
@@ -672,37 +672,57 @@ def self.ipv6_mac(intf)
   #       is no concurrent use of the same locals and this is safe.
   def self.tcp_socket_pair
     lsock   = nil
+    last_child_error = nil
+    accept_timeout = 10
     rsock   = nil
     laddr   = '127.0.0.1'
     lport   = 0
     threads = []
     mutex   = ::Mutex.new
 
-    threads << Rex::ThreadFactory.spawn('TcpSocketPair', false) {
+    threads << Rex::ThreadFactory.spawn('TcpSocketPair', false) do
       server = nil
-      mutex.synchronize {
-        threads << Rex::ThreadFactory.spawn('TcpSocketPairClient', false) {
-          mutex.synchronize {
-            rsock = ::TCPSocket.new( laddr, lport )
-          }
-        }
-        server = ::TCPServer.new(laddr, 0)
-        if (server.getsockname =~ /127\.0\.0\.1:/)
-          # JRuby ridiculousness
-          caddr, lport = server.getsockname.split(":")
-          caddr = caddr[1,caddr.length]
-          lport = lport.to_i
+      begin
+        mutex.synchronize do
+          threads << Rex::ThreadFactory.spawn('TcpSocketPairClient', false) do
+            mutex.synchronize do
+              begin
+                rsock = ::TCPSocket.new( laddr, lport )
+              rescue => e
+                last_child_error = "#{e.class} - #{e.message}"
+                raise
+              end
+            end
+          end
+          server = ::TCPServer.new(laddr, 0)
+          if (server.getsockname =~ /127\.0\.0\.1:/)
+            # JRuby ridiculousness
+            caddr, lport = server.getsockname.split(":")
+            caddr = caddr[1,caddr.length]
+            lport = lport.to_i
+          else
+            # Sane implementations where Socket#getsockname returns a
+            # sockaddr
+            lport, caddr = ::Socket.unpack_sockaddr_in( server.getsockname )
+          end
+        end
+
+        readable, _writable, _errors = ::IO.select([server], nil, nil, accept_timeout)
+        if readable && readable.any?
+          lsock, _ = server.accept_nonblock
         else
-          # Sane implementations where Socket#getsockname returns a
-          # sockaddr
-          lport, caddr = ::Socket.unpack_sockaddr_in( server.getsockname )
+          raise RuntimeError, "rsock didn't connect in #{accept_timeout} seconds"
         end
-      }
-      lsock, _ = server.accept
-      server.close
-    }
+      ensure
+        server.close if server
+      end
+    end
 
-    threads.each { |t| t.join }
+    threads.each.with_index do |thread, i|
+      thread.join
+    rescue => e
+      raise "Thread #{i} - error #{e} - last child error: #{last_child_error}"
+    end
 
     return [lsock, rsock]
   end
diff --git a/spec/rex/socket_spec.rb b/spec/rex/socket_spec.rb
index ac8db69..3d2d3af 100644
--- a/spec/rex/socket_spec.rb
+++ b/spec/rex/socket_spec.rb
@@ -4,6 +4,44 @@
 
 RSpec.describe Rex::Socket do
 
+  describe '.tcp_socket_pair' do
+    let(:mock_thread_factory) do
+      double :mock_thread_factory
+    end
+
+    before(:each) do
+      stub_const('Rex::ThreadFactory', mock_thread_factory)
+      # Fallback implementation from https://github.com/rapid7/metasploit-framework/blob/30e66c43a4932df922d7f1d986fb98387bd0ab1a/lib/rex/thread_factory.rb#L27-L37
+      allow(mock_thread_factory).to receive(:spawn) do |name, crit, *args, &block|
+        if block
+          t = ::Thread.new(*args){ |*args_copy| block.call(*args_copy) }
+        else
+          t = ::Thread.new(*args)
+        end
+        t[:tm_name] = name
+        t[:tm_crit] = crit
+        t[:tm_time] = ::Time.now
+        t[:tm_call] = caller
+        t
+      end
+    end
+
+    it 'creates two socket pairs' do
+      lsock, rsock = described_class.tcp_socket_pair
+      lsock.extend(Rex::IO::Stream)
+      rsock.extend(Rex::IO::Stream)
+
+      expect(lsock.closed?).to be(false)
+      expect(rsock.closed?).to be(false)
+
+      lsock.close
+      rsock.close
+
+      expect(lsock.closed?).to be(true)
+      expect(rsock.closed?).to be(true)
+    end
+  end
+
   describe '.addr_itoa' do
 
     context 'with explicit v6' do