TcpServerChannel#accept blocks indefinitely by default, causing hangs on slow sessions

[dwelch-r7]

Summary

Rex::Post::Meterpreter::Extensions::Stdapi::Net::SocketSubsystem::TcpServerChannel#accept defaults to an infinite timeout when no Timeout option is passed. This causes callers to hang indefinitely if the meterpreter is slow to relay the channel-open notification — particularly visible on 32-bit Windows meterpreter sessions on resource-constrained hosts.

Root Cause

In lib/rex/post/meterpreter/extensions/stdapi/net/socket_subsystem/tcp_server_channel.rb:

def accept(opts = {})
  timeout = opts['Timeout']
  if (timeout.nil? || timeout <= 0)
    timeout = 0  # Timeout.timeout(0) means NO timeout
  end

  result = nil
  begin
    ::Timeout.timeout(timeout) {
      result = _accept
    }
  rescue Timeout::Error
  end

  result
end

_accept calls @@server_channels[self].deq(false) — a blocking Queue#deq. With timeout = 0, Timeout.timeout(0) effectively disables the timeout. If the meterpreter never sends the COMMAND_ID_STDAPI_NET_TCP_CHANNEL_OPEN notification (or sends it slowly), the caller blocks forever.

How the accept flow works

1. Framework creates a TCP server channel on the meterpreter via TcpServerChannel.open
2. A client connects to the server at the OS level (TCP handshake completes)
3. Meterpreter detects the new connection and sends COMMAND_ID_STDAPI_NET_TCP_CHANNEL_OPEN back to the framework via TLV
4. Framework's TcpServerChannel.request_handler receives the packet, creates a TcpClientChannel, and enqueues it into @@server_channels[self] (a Queue)
5. accept dequeues the client channel and returns the socket

The race condition exists between steps 2 and 4. On slow/loaded sessions, there can be a significant delay between the TCP connection completing and the framework receiving the notification.

Impact

• CI: The post/test/socket_channels acceptance test calls tcp_server_socket_pair which used server.accept with no timeout. On windows-2022 CI runners with 32-bit meterpreter, the accept occasionally blocks for the entire 50-minute CI job timeout, causing the job to be cancelled.
• Production use: Any module or user script calling server.accept without explicitly passing a Timeout option will block indefinitely if the meterpreter session is slow or dies mid-operation. There's no way to recover without killing the Ruby thread.

Proposed Fix

Change the default timeout behavior so accept uses a reasonable finite default rather than blocking forever:

def accept(opts = {})
  timeout = opts['Timeout']
  if timeout.nil? || timeout <= 0
    timeout = 300  # Default 5 minutes instead of infinite
  end

  result = nil
  begin
    ::Timeout.timeout(timeout) {
      result = _accept
    }
  rescue Timeout::Error
  end

  result
end

Alternatively, expose accept_nonblock more prominently and update callers to use a poll/retry pattern.

Considerations

• Changing the default timeout may affect existing modules that rely on accept blocking indefinitely (e.g., pivot listeners that wait for connections over long periods). An audit of callers would be needed.
• A safer approach might be to add a default timeout constant (e.g., DEFAULT_ACCEPT_TIMEOUT = 300) that callers can override, while logging a warning when the timeout is reached.
• The request_handler class method uses a class variable (@@server_channels) which could have thread-safety concerns under heavy load, though the Queue itself is thread-safe.

Related

• windows_meterpreter windows-2022 builds are flaky #21611 — windows_meterpreter windows-2022 builds are flaky
• Fix file removal logic to ensure proper cleanup and return status #21703
• lib/rex/post/meterpreter/extensions/stdapi/net/socket_subsystem/tcp_server_channel.rb
• test/modules/post/test/socket_channels.rb (workaround: retry loop with bounded timeout added in Fix file removal logic to ensure proper cleanup and return status #21703)

[kx7m2qd]

Looked at #21705 which is linked to close this — the core change (finite default instead of 0/infinite) looks right, but a few things from the "Considerations" section above don't seem to be addressed in that diff yet:

1. Magic number vs. constant — you'd suggested a named DEFAULT_ACCEPT_TIMEOUT = 300 rather than an inline 300, so it's documented and easy to tune in one place later.
2. Silent timeout — Timeout::Error is still just rescued and nil returned, no warning logged when the default timeout is hit.
3. Caller audit — not clear from the PR whether callers relying on indefinite blocking (e.g. pivot listeners) were checked.

@dwelch-r7, @hanu-14 flagging these in case they're still in progress, happy to help with the caller audit if useful.