Skip to content

Commit

Permalink
Add initial layout cops for the module super hash
Browse files Browse the repository at this point in the history
  • Loading branch information
adfoster-r7 committed Mar 6, 2020
1 parent 513338c commit bfd284b
Show file tree
Hide file tree
Showing 8 changed files with 690 additions and 23 deletions.
89 changes: 68 additions & 21 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,16 @@
AllCops:
TargetRubyVersion: 2.4

require:
- ./lib/rubocop/cop/layout/module_hash_on_new_line.rb
- ./lib/rubocop/cop/layout/module_description_indentation.rb

Layout/ModuleHashOnNewLine:
Enabled: true

Layout/ModuleDescriptionIndentation:
Enabled: true

Metrics/ClassLength:
Description: 'Most Metasploit modules are quite large. This is ok.'
Enabled: true
Expand Down Expand Up @@ -59,6 +69,25 @@ Style/Documentation:
Exclude:
- 'modules/**/*'

Layout/FirstArgumentIndentation:
Enabled: true
EnforcedStyle: consistent
Description: 'Useful for the module hash to be indented consistently'

Layout/ArgumentAlignment:
Enabled: true
EnforcedStyle: with_first_argument
Description: 'Useful for the module hash to be indented consistently'

Layout/FirstHashElementIndentation:
Enabled: true
EnforcedStyle: consistent
Description: 'Useful for the module hash to be indented consistently'

Layout/FirstHashElementLineBreak:
Enabled: true
Description: 'Enforce consistency by breaking hash elements on to new lines'

Layout/SpaceInsideArrayLiteralBrackets:
Enabled: false
Description: 'Almost all module metadata have space in brackets'
Expand Down Expand Up @@ -93,26 +122,26 @@ Style/TrailingCommaInArrayLiteral:

Metrics/LineLength:
Description: >-
Metasploit modules often pattern match against very
long strings when identifying targets.
Metasploit modules often pattern match against very
long strings when identifying targets.
Enabled: true
Max: 180

Metrics/BlockLength:
Enabled: true
Description: >-
While the style guide suggests 10 lines, exploit definitions
often exceed 200 lines.
While the style guide suggests 10 lines, exploit definitions
often exceed 200 lines.
Max: 300

Metrics/MethodLength:
Enabled: true
Description: >-
While the style guide suggests 10 lines, exploit definitions
often exceed 200 lines.
While the style guide suggests 10 lines, exploit definitions
often exceed 200 lines.
Max: 300

Naming/MethodParameterName:
Naming/MethodParameterName:
Enabled: true
Description: 'Whoever made this requirement never looked at crypto methods, IV'
MinNameLength: 2
Expand All @@ -126,13 +155,10 @@ Style/NumericLiterals:
Enabled: false
Description: 'This often hurts readability for exploit-ish code.'

Layout/HashAlignment:
Enabled: false
Description: 'aligning info hashes to match these rules is almost impossible to get right'

Layout/EmptyLines:
Enabled: false
Description: 'these are used to increase readability'
Layout/FirstArrayElementIndentation:
Enabled: true
EnforcedStyle: consistent
Description: 'Useful to force values within the register_options array to have sane indentation'

Layout/EmptyLinesAroundClassBody:
Enabled: false
Expand All @@ -142,19 +168,24 @@ Layout/EmptyLinesAroundMethodBody:
Enabled: false
Description: 'these are used to increase readability'

Layout/ParameterAlignment:
Layout/ExtraSpacing:
Description: 'Do not use unnecessary spacing.'
Enabled: true
EnforcedStyle: 'with_fixed_indentation'
Description: 'initialize method of every module has fixed indentation for Name, Description, etc'
# When true, allows most uses of extra spacing if the intent is to align
# things with the previous or next line, not counting empty lines or comment
# lines.
AllowForAlignment: false
# When true, allows things like 'obj.meth(arg) # comment',
# rather than insisting on 'obj.meth(arg) # comment'.
# If done for alignment, either this OR AllowForAlignment will allow it.
AllowBeforeTrailingComments: false
# When true, forces the alignment of `=` in assignments on consecutive lines.
ForceEqualSignAlignment: false

Style/For:
Enabled: false
Description: 'if a module is written with a for loop, it cannot always be logically replaced with each'

Style/StringLiterals:
Enabled: false
Description: 'Single vs double quote fights are largely unproductive.'

Style/WordArray:
Enabled: false
Description: 'Metasploit prefers consistent use of []'
Expand All @@ -163,6 +194,22 @@ Style/IfUnlessModifier:
Enabled: false
Description: 'This style might save a couple of lines, but often makes code less clear'

Style/PercentLiteralDelimiters:
Description: 'Use `%`-literal delimiters consistently.'
Enabled: true
# Specify the default preferred delimiter for all types with the 'default' key
# Override individual delimiters (even with default specified) by specifying
# an individual key
PreferredDelimiters:
default: ()
'%i': '[]'
'%I': '[]'
'%r': '{}'
'%w': '[]'
'%W': '[]'
'%q': '{}' # Chosen for module descriptions as () are frequently used characters, whilst {} are rarely used
VersionChanged: '0.48.1'

Style/RedundantBegin:
Exclude:
# this pattern is very common and somewhat unavoidable
Expand Down
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ group :development, :test do
# environment is development
gem 'rspec-rails'
gem 'rspec-rerun'
gem 'rubocop'
gem 'swagger-blocks'
end

Expand Down
18 changes: 18 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ GEM
arel (6.0.4)
arel-helpers (2.11.0)
activerecord (>= 3.1.0, < 7)
ast (2.4.0)
aws-eventstream (1.0.3)
aws-partitions (1.279.0)
aws-sdk-core (3.90.1)
Expand Down Expand Up @@ -185,6 +186,7 @@ GEM
http_parser.rb (0.6.0)
i18n (0.9.5)
concurrent-ruby (~> 1.0)
jaro_winkler (1.5.4)
jmespath (1.4.0)
jsobfu (0.4.2)
rkelly-remix
Expand Down Expand Up @@ -242,6 +244,9 @@ GEM
openvas-omp (0.0.4)
packetfu (1.1.13)
pcaprub
parallel (1.19.1)
parser (2.7.0.2)
ast (~> 2.4.0)
patch_finder (1.0.2)
pcaprub (0.13.0)
pdf-reader (2.4.0)
Expand Down Expand Up @@ -281,6 +286,7 @@ GEM
activesupport (= 4.2.11.1)
rake (>= 0.8.7)
thor (>= 0.18.1, < 2.0)
rainbow (3.0.0)
rake (13.0.1)
rb-readline (0.5.5)
recog (2.3.7)
Expand Down Expand Up @@ -333,6 +339,7 @@ GEM
rex-text (0.2.24)
rex-zip (0.1.3)
rex-text
rexml (3.2.4)
rkelly-remix (0.0.7)
rspec (3.9.0)
rspec-core (~> 3.9.0)
Expand All @@ -357,7 +364,16 @@ GEM
rspec-rerun (1.1.0)
rspec (~> 3.0)
rspec-support (3.9.2)
rubocop (0.80.0)
jaro_winkler (~> 1.5.1)
parallel (~> 1.10)
parser (>= 2.7.0.1)
rainbow (>= 2.2.2, < 4.0)
rexml
ruby-progressbar (~> 1.7)
unicode-display_width (>= 1.4.0, < 1.7)
ruby-macho (2.2.0)
ruby-progressbar (1.10.1)
ruby-rc4 (0.1.5)
ruby_smb (1.1.0)
bindata
Expand Down Expand Up @@ -392,6 +408,7 @@ GEM
thread_safe (~> 0.1)
tzinfo-data (1.2019.3)
tzinfo (>= 1.0.0)
unicode-display_width (1.6.1)
warden (1.2.7)
rack (>= 1.0)
websocket-driver (0.7.1)
Expand All @@ -417,6 +434,7 @@ DEPENDENCIES
redcarpet
rspec-rails
rspec-rerun
rubocop
simplecov
sqlite3 (~> 1.3.0)
swagger-blocks
Expand Down
78 changes: 78 additions & 0 deletions lib/rubocop/cop/layout/module_description_indentation.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
module RuboCop
module Cop
module Layout
class ModuleDescriptionIndentation < Cop
include Alignment

MSG = "Module descriptions should be properly aligned to the 'Description' key, and within %q{ ... }"

def_node_matcher :find_update_info_node, <<~PATTERN
(def :initialize _args (begin (super $(send nil? {:update_info :merge_info} (lvar :info) (hash ...))) ...))
PATTERN

def_node_matcher :find_nested_update_info_node, <<~PATTERN
(def :initialize _args (super $(send nil? {:update_info :merge_info} (lvar :info) (hash ...)) ...))
PATTERN

def on_def(node)
update_info_node = find_update_info_node(node) || find_nested_update_info_node(node)
return if update_info_node.nil?

hash = update_info_node.arguments.find { |argument| hash_arg?(argument) }
hash.each_pair do |key, value|
if key.value == "Description"
if requires_correction?(key, value)
add_offense(value, location: :end)
end
end
end
end

def autocorrect(description_value)
lambda do |corrector|
description_key = description_value.parent.key
new_content = indent_description_value_correctly(description_key, description_value)

corrector.replace(description_value.source_range, new_content)
end
end

private

def requires_correction?(description_key, description_value)
return false if description_value.single_line?

current_content = description_value.source
expected_content = indent_description_value_correctly(description_key, description_value)
expected_content != current_content
end

def indent_description_value_correctly(description_key, description_value)
content_whitespace = indentation(description_key)
final_line_whitespace = offset(description_key)

description_content = description_value.source.lines[1...-1]
indented_description = description_content.map do |line|
cleaned_content = line.strip
if cleaned_content.empty?
"\n"
else
"#{content_whitespace}#{cleaned_content}\n"
end
end.join

new_literal = "%q{\n"
new_literal <<= indented_description
new_literal <<= final_line_whitespace
new_literal <<= '}'

new_literal
end

def hash_arg?(node)
node.type == :hash
end
end
end
end
end
87 changes: 87 additions & 0 deletions lib/rubocop/cop/layout/module_hash_on_new_line.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
module RuboCop
module Cop
module Layout
class ModuleHashOnNewLine < Cop
include Alignment

MSG = "%<name>s should start on its own line"
MISSING_NEW_LINE_MSG = "A new line is missing"

def_node_matcher :find_update_info_node, <<~PATTERN
(def :initialize _args (begin (super $(send nil? {:update_info :merge_info} (lvar :info) (hash ...))) ...))
PATTERN

def_node_matcher :find_nested_update_info_node, <<~PATTERN
(def :initialize _args (super $(send nil? {:update_info :merge_info} (lvar :info) (hash ...)) ...))
PATTERN

def on_def(node)
update_info_node = find_update_info_node(node) || find_nested_update_info_node(node)
return if update_info_node.nil?

unless begins_its_line?(update_info_node.source_range)
add_offense(update_info_node, location: :begin)
end

# Ensure every argument to update_info is on its own line, i.e. info and the hash arguments
update_info_node.arguments.each do |argument|
unless begins_its_line?(argument.source_range)
add_offense(argument)
end
end

if missing_new_line_after_parenthesis?(update_info_node)
add_offense(update_info_node, location: :end, message: MISSING_NEW_LINE_MSG)
end
end

def autocorrect(node)
lambda do |corrector|
if merge_function?(node) && missing_new_line_after_parenthesis?(node)
# Ensure there's always a new line after `update_info(...)` to avoid `))` at the end of the `super(update_info` call
corrector.replace(node.source_range.end, "\n#{offset(node.parent)}")
else
# Force a new line, and indent to the parent node. Other Layout rules will correct the positioning.
corrector.replace(node.source_range, "\n#{indentation(node.parent)}#{node.source}")
end
end
end

private

def message(node)
if update_info?(node)
format(MSG, name: :update_info)
elsif merge_info?(node)
format(MSG, name: :merge_info)
elsif info_arg?(node)
format(MSG, name: :info)
else
format(MSG, name: :argument)
end
end

def merge_function?(node)
update_info?(node) || merge_info?(node)
end

def update_info?(node)
node.type == :send && node.method_name == :update_info
end

def merge_info?(node)
node.type == :send && node.method_name == :merge_info
end

def info_arg?(node)
node.type == :lvar && node.children[0] == :info
end

def missing_new_line_after_parenthesis?(update_info_node)
super_call = update_info_node.parent
super_call.source.end_with? '))'
end
end
end
end
end
Loading

0 comments on commit bfd284b

Please sign in to comment.