Implement jvazquez-r7 comments

modules/auxiliary/scanner/scada/modbusclient.rb

@@ -8,47 +8,42 @@ require 'msf/core'
 class Metasploit3 < Msf::Auxiliary
 
   include Msf::Exploit::Remote::Tcp
-  include Rex::Socket::Tcp
 
   def initialize(info = {})
     super(update_info(info,
-      'Name'          => 'Modbus client, reloaded.',
+      'Name'          => 'Modbus Client Utility',
       'Description'   => %q{
         This module allows reading and writing data to a PLC using the Modbus protocol.
-
         This module is based on the 'modiconstop.rb' Basecamp module from
         DigitalBond, as well as the mbtget perl script.
       },
       'Author'         =>
         [
-          'EsMnemon <esm [at] mnemonic.no>', # original write-only module
-          'Arnaud SOULLIE  <arnaud.soullie[at]solucom.fr>', # new code that allows read/write
+          'EsMnemon <esm[at]mnemonic.no>', # original write-only module
+          'Arnaud SOULLIE  <arnaud.soullie[at]solucom.fr>' # new code that allows read/write
         ],
       'License'        => MSF_LICENSE,
+      'Actions'        => [AuxiliaryAction.new('READ_COIL', {}),
+                          AuxiliaryAction.new('WRITE_COIL', {}),
+                          AuxiliaryAction.new('READ_REGISTER', {}),
+                          AuxiliaryAction.new('WRITE_REGISTER', {})
+                          ]
       ))
     register_options(
       [
-        OptEnum.new("MODE", [true, 'Command', "READ_REGISTER",
-          [
-            "READ_REGISTER",
-            "READ_COIL",
-            "WRITE_REGISTER",
-            "WRITE_COIL"
-          ]
-        ]),
         Opt::RPORT(502),
-        OptInt.new('DATA', [false, "Data to write (WRITE_COIL and WRITE_REGISTER modes only)", 0xBEEF]),
-        OptInt.new('DATA_ADDRESS', [true, "Modbus data address", 0]),
+        OptInt.new('DATA', [false, "Data to write (WRITE_COIL and WRITE_REGISTER modes only)"]),
+        OptInt.new('DATA_ADDRESS', [true, "Modbus data address"]),
         OptInt.new('UNIT_NUMBER', [false, "Modbus unit number", 1]),
       ], self.class)
 
   end
 
   # a wrapper just to be sure we increment the counter
-  def sendframe(payload)
+  def send_frame(payload)
     sock.put(payload)
-    @modbuscounter += 1
-    r = sock.recv(65535, 0.1)
+    @modbus_counter += 1
+    r = sock.get
     return r
   end
 
@@ -59,13 +54,13 @@ class Metasploit3 < Msf::Auxiliary
     payload += [datastore['DATA_ADDRESS']].pack("n")
     payload += [1].pack("n")
 
-    packetdata = ""
-    packetdata += [@modbuscounter].pack("n")
-    packetdata += "\x00\x00\x00" #dunno what these are
-    packetdata += [payload.size].pack("c") # size byte
-    packetdata += payload
+    packet_data = ""
+    packet_data += [@modbus_counter].pack("n")
+    packet_data += "\x00\x00\x00" #dunno what these are
+    packet_data += [payload.size].pack("c") # size byte
+    packet_data += payload
 
-    return packetdata
+    return packet_data
   end
 
   def make_write_coil_payload(data)
@@ -76,13 +71,13 @@ class Metasploit3 < Msf::Auxiliary
     payload += [data].pack("c")
     payload += "\x00"
 
-    packetdata = ""
-    packetdata += [@modbuscounter].pack("n")
-    packetdata += "\x00\x00\x00" #dunno what these are
-    packetdata += [payload.size].pack("c") # size byte
-    packetdata += payload
+    packet_data = ""
+    packet_data += [@modbus_counter].pack("n")
+    packet_data += "\x00\x00\x00" #dunno what these are
+    packet_data += [payload.size].pack("c") # size byte
+    packet_data += payload
 
-    return packetdata
+    return packet_data
   end
 
     def make_write_register_payload(data)
@@ -92,27 +87,27 @@ class Metasploit3 < Msf::Auxiliary
     payload += [datastore['DATA_ADDRESS']].pack("n")
     payload += [data].pack("n")
 
-    packetdata = ""
-    packetdata += [@modbuscounter].pack("n")
-    packetdata += "\x00\x00\x00" #dunno what these are
-    packetdata += [payload.size].pack("c") # size byte
-    packetdata += payload
+    packet_data = ""
+    packet_data += [@modbus_counter].pack("n")
+    packet_data += "\x00\x00\x00" #dunno what these are
+    packet_data += [payload.size].pack("c") # size byte
+    packet_data += payload
 
-    return packetdata
+    return packet_data
   end
 
   def run
-    @modbuscounter = 0x0000 # used for modbus frames
+    @modbus_counter = 0x0000 # used for modbus frames
     connect
-    case datastore['MODE']
+    case datastore['ACTION']
     when "READ_COIL"
       @function_code = 1
-      response = sendframe(make_read_payload)
+      response = send_frame(make_read_payload)
       print_good("Coil value at address #{datastore['DATA_ADDRESS']} : " + response.reverse.unpack("c").to_s.gsub('[', '').gsub(']', ''))
 
     when "READ_REGISTER"
       @function_code = 3
-      response = sendframe(make_read_payload)
+      response = send_frame(make_read_payload)
       value = response.split[0][9..10].to_s.unpack("n").to_s.gsub('[', '').gsub(']','')
       print_good("Register value at address #{datastore['DATA_ADDRESS']} : " + value)
 
@@ -126,7 +121,7 @@ class Metasploit3 < Msf::Auxiliary
         print_error("Data value must be 0 or 1 in WRITE_COIL mode")
         exit
       end
-      response = sendframe(make_write_coil_payload(data))
+      response = send_frame(make_write_coil_payload(data))
       print_good("Value #{datastore['DATA']} successfully written at coil address #{datastore['DATA_ADDRESS']}")
 
     when "WRITE_REGISTER"
@@ -135,11 +130,11 @@ class Metasploit3 < Msf::Auxiliary
         print_error("Data to write must be an integer between 0 and 65535 in WRITE_REGISTER mode")
         exit
       end
-      response = sendframe(make_write_register_payload(datastore['DATA']))
+      response = send_frame(make_write_register_payload(datastore['DATA']))
       print_good("Value #{datastore['DATA']} successfully written at registry address #{datastore['DATA_ADDRESS']}")
 
     else
-      print_error("Invalid MODE")
+      print_error("Invalid ACTION")
       return
     end
   end