Land rapid7#8301, Improve msftidy checks

Author: Brent Cook

modules/auxiliary/scanner/http/brute_dirs.rb

@@ -3,8 +3,6 @@
 # Current source: https://github.com/rapid7/metasploit-framework
 ##
 
-
-require 'msf/core'
 require 'enumerable'
 
 class MetasploitModule < Msf::Auxiliary
@@ -29,7 +27,7 @@ def initialize(info = {})
       [
         OptString.new('PATH', [ true,  "The path to identify directories", '/']),
         OptString.new('FORMAT', [ true,  "The expected directory format (a alpha, d digit, A upperalpha)", 'a,aa,aaa'])
-      ], self.class)
+      ])
 
     register_advanced_options(
       [
@@ -40,7 +38,7 @@ def initialize(info = {})
         ),
         OptBool.new('NoDetailMessages', [ false, "Do not display detailed test messages", true ]),
         OptInt.new('TestThreads', [ true, "Number of test threads", 25])
-      ], self.class)
+      ])
 
   end
 

spec/file_fixtures/modules/auxiliary/auxiliary_tidy.rb

@@ -3,8 +3,6 @@
 # Current source: https://github.com/rapid7/metasploit-framework
 ##
 
-require 'msf/core'
-
 class MetasploitModule < Msf::Auxiliary
   def initialize(info = {})
     super(

tools/dev/msftidy.rb

@@ -12,16 +12,6 @@
 
 CHECK_OLD_RUBIES = !!ENV['MSF_CHECK_OLD_RUBIES']
 SUPPRESS_INFO_MESSAGES = !!ENV['MSF_SUPPRESS_INFO_MESSAGES']
-TITLE_WHITELIST = %w{
-  a an and as at avserve callmenum configdir connect debug docbase dtspcd
-  execve file for from getinfo goaway gsad hetro historysearch htpasswd ibstat
-  id in inetd iseemedia jhot libxslt lmgrd lnk load main map migrate mimencode
-  multisort name net netcat nodeid ntpd nttrans of on onreadystatechange or
-  ovutil path pbot pfilez pgpass pingstr pls popsubfolders prescan readvar
-  relfile rev rexec rlogin rsh rsyslog sa sadmind say sblistpack spamd
-  sreplace tagprinter the tnftp to twikidraw udev uplay user username via
-  welcome with ypupdated zsudo
-}
 
 if CHECK_OLD_RUBIES
   require 'rvm'
@@ -215,6 +205,18 @@ def check_ref_identifiers
     end
   end
 
+  def check_self_class
+    in_register = false
+    @lines.each do |line|
+      (in_register = true) if line =~ /^\s*register_(?:advanced_)?options/
+      (in_register = false) if line =~ /^\s*end/
+      if in_register && line =~ /\],\s*self\.class\s*\)/
+        warn('Explicitly using self.class in register_* is not necessary')
+        break
+      end
+    end
+  end
+
   # See if 'require "rubygems"' or equivalent is used, and
   # warn if so.  Since Ruby 1.9 this has not been necessary and
   # the framework only suports 1.9+
@@ -227,6 +229,15 @@ def check_rubygems
     end
   end
 
+  def check_msf_core
+    @lines.each do |line|
+      if line_has_require?(line, 'msf/core')
+        warn('Explicitly requiring/loading msf/core is not necessary')
+        break
+      end
+    end
+  end
+
   # Does the given line contain a require/load of the specified library?
   def line_has_require?(line, lib)
     line =~ /^\s*(require|load)\s+['"]#{lib}['"]/
@@ -447,19 +458,6 @@ def check_disclosure_date
     end
   end
 
-  def check_title_casing
-    if @source =~ /["']Name["'][[:space:]]*=>[[:space:]]*['"](.+)['"],*$/
-      words = $1.split
-      words.each do |word|
-        if TITLE_WHITELIST.include?(word)
-          next
-        elsif word =~ /^[a-z]+$/
-          warn("Suspect capitalization in module title: '#{word}'")
-        end
-      end
-    end
-  end
-
   def check_bad_terms
     # "Stack overflow" vs "Stack buffer overflow" - See explanation:
     # http://blogs.technet.com/b/srd/archive/2009/01/28/stack-overflow-stack-exhaustion-not-the-same-as-stack-buffer-overflow.aspx
@@ -701,15 +699,16 @@ def run_checks
     check_shebang
     check_nokogiri
     check_rubygems
+    check_msf_core
     check_ref_identifiers
+    check_self_class
     check_old_keywords
     check_verbose_option
     check_badchars
     check_extname
     check_old_rubies
     check_ranking
     check_disclosure_date
-    check_title_casing
     check_bad_terms
     check_bad_super_class
     check_bad_class_name