rapid7/metasploit-framework — comment on commit e204923966ecee6efb8aaf8e79df3e81e22a4913
File:  line 0
By todb-r7:

My initial first pass on this added a proper OptPath option for the KB list, and fixed a lot of the indenting, whereupon I found some pretty serious structural problems with the module.

The biggest hint that something is terribly wrong with the design of any chunk of Ruby code is the existence of the long chain of `end` statements at the end of the module (there are nine on this one, in a row).

For starters, all the functions should not be defined in `def run`. While allowed, and can be neat for metaprogramming, there is no need for this here. The module should be basically:

def run
do_a_thing()
do_something_else()
report_on_results()
end

def do_a_thing
# Things happen here, etc.
end

Not like this:

def run
def do_a_thing
# Things happen here, etc.
end

do_a_thing()
do_something_else()
report_on_results()
end

I didn't tackle this reorganization, Harshal should. There are

I also changed some functionality where the module would abort if the user already has system privs. While detailing local priv escalation vulns in this case is probably useless practically speaking, it is useful information for most pentest / auditor types.

I can fix up spelling and grammar later, it's not that big of a deal right now.

More comments in line in a sec.