Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Packrat: a post module to gather artifacts from a multitude of applications #11719

Closed
wants to merge 10 commits into from
Closed

Packrat: a post module to gather artifacts from a multitude of applications #11719

wants to merge 10 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 12, 2019

… it can gather from applications on end user systems.

Artifacts include: chat logins and logs, browser logins and history and
cookies, email logins and emails sent and received and deleted, contacts
and many others. These artifacts are collected from applications
including: 12 browsers, 13 chat/IM/IRC applications, 6 email clients and
1 game.

These artifacts are then scraped for credentials,

The use case for this post-exploitation module is to specify the types
of artefacts you are interesed in, to gather the relevent files
depending on your aims.

Tell us what this change does. If you're fixing a bug, please mention
the github issue number.

Please ensure you are submitting from a unique branch in your repository to master in Rapid7's.

Verification

List the steps needed to make sure this thing works

  • Start msfconsole
  • use exploit/windows/smb/ms08_067_netapi
  • ...
  • Verify the thing does what it should
  • Verify the thing does not do what it should not
  • Document the thing and how it works (Example)

… it can gather from applications on end user systems.

Artifacts include: chat logins and logs, browser logins and history and
cookies, email logins and emails sent and received and deleted, contacts
and many others. These artifacts are collected from applications
including: 12 browsers, 13 chat/IM/IRC applications, 6 email clients and
1 game.

These artifacts are then scraped for credentials,

The use case for this post-exploitation module is to specify the types
of artefacts you are interesed in, to gather the relevent files
depending on your aims.
@wvu wvu changed the title This post exploitation module has an extensive list of artefacts that… Add post module to gather artifacts from a multitude of applications Apr 12, 2019
Copy link
Contributor

@wvu wvu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please review and merge https://github.com/danhallsworth1/metasploit-framework/pull/1.

You will also want to run RuboCop against this module with our .rubocop.yml and clean up the most reasonable reported issues. You should write a module doc as well.

You can find our contributing guide here. We more or less follow the Ruby style guide.

Thanks!

This merge makes the changes on pull request 11719. Module has been
tested and still works and also passes Robocop
@ghost
Copy link
Author

ghost commented Apr 19, 2019

Hi I forgot to add the data/packrat/artifacts.json directory in this pull request, I think that's why the checks have failed. I just put the 'artifact.json' file in the 'metasploit/data' directory. Do you need me to submit it again in the correct directory @wvu-r7

@wvu
Copy link
Contributor

wvu commented Apr 19, 2019

@danhallsworth1: You shouldn't need to do that. The merge took care of it. Check out the diff: https://github.com/rapid7/metasploit-framework/pull/11719/files. :)

The RSpec failures look unrelated. Do we know anything about this, @jmartin-r7?

@ghost
Copy link
Author

ghost commented Apr 19, 2019

ah amazing so is there anything I else I need to do now then? @wvu-r7

@wvu
Copy link
Contributor

wvu commented Apr 19, 2019

Please review and merge danhallsworth1#1.

You will also want to run RuboCop against this module with our .rubocop.yml and clean up the most reasonable reported issues. You should write a module doc as well.

You can find our contributing guide here. We more or less follow the Ruby style guide.

Thanks!

I would use a consistent style in the code. Now that the large hash is well-formed JSON and out of the way, we can focus on improving the functional code within the module. I suggest leveraging RuboCop.

Note that better code, especially when documented (as requested), leads to more maintainable code. And we will have to maintain it. :-)

Let me know if you need any assistance. Thanks!


#used to grab files for each user on the remote host.
grab_user_profiles.each do |userprofile|
APPLICATION_ARRAY.each {|app_loop|
Copy link
Contributor

@wvu wvu Apr 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an example of what I mean by consistent, you are using do for the outside block but { for the inside block, and both blocks are multiline. Please use do as per https://github.com/rubocop-hq/ruby-style-guide#single-line-blocks.

I don't care so much about what the "right" style is so long as your code is consistent and clean.

Good coding is language-agnostic.

@ghost
Copy link
Author

ghost commented Apr 19, 2019 via email

grab_user_profiles.each do |userprofile|
APPLICATION_ARRAY.each {|app_loop|
download(userprofile, app_loop)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an example of what I mean by clean, this blank line should be removed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi i've made some of these changes and ran Robocop against it fixing the main issues its highlighted. I've got 2 uni exams coming up soon so wont be able to make the changes to this as soon as id like, but i'll still be working on it hope that's okay @wvu-r7

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally fine with that. Thanks for taking the time to contribute. Thanks to your collaborators, too! And good luck on the exams. :)

@wvu wvu changed the title Add post module to gather artifacts from a multitude of applications Packrat: a post module to gather artifacts from a multitude of applications Apr 19, 2019
@wvu wvu self-assigned this Apr 24, 2019
@ghost
Copy link
Author

ghost commented May 22, 2019

Hi I’ve made these changes now and have made the documentation, ready to submit. What’s are the commands I need to do to submit these changes?

@timwr
Copy link
Contributor

timwr commented May 22, 2019

git add 
git commit
git push

? :trollface:

@jrobles-r7
Copy link
Contributor

To update this PR push to your branch danhallsworth1:packrat_post_module.

@wvu
Copy link
Contributor

wvu commented May 22, 2019

No, you don't want to submit a new PR. I'll close that so you can add the commits here.

@wvu
Copy link
Contributor

wvu commented May 22, 2019

I do something like this:

git add -p
git commit
git push origin packrat_post_module

throughout the code e.g. loops, and made the changes suggested through
robocop
@ghost
Copy link
Author

ghost commented May 22, 2019

I do something like this:

git add -p
git commit
git push origin packrat_post_module

Hi thanks those commands helped a lot, got everything submitted now thanks for the help

Copy link
Contributor

@wvu wvu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have two modules in this PR. Please consolidate your changes into one.


Users can enter APPLICATIONS to extract from example output shown below for email service incredimail

msf post(windows/gather/credentials/packrat_credentials) > set SESSION 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll want to put code in code blocks.

Copy link
Contributor

@cbrnrd cbrnrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty solid overall. What I commented is mostly small things (verbose context, code style, grammar, etc.) Thanks for taking the time to write this!

'Description' => %q{
This module extracts artifcats from a large list of applications
and can extract credentials storing content in loot. Full list in
module documentation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Full list in module documentation. -> The full list of supported applications is in the module documentation.


register_options(
[
OptRegexp.new('REGEX', [false, 'Match a regular expression', '^password']),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarify what's being matched by the regex

OptRegexp.new('REGEX', [false, 'Match a regular expression', '^password']),
OptBool.new('STORE_LOOT', [false, 'Store artifacts into loot database (otherwise, only download)', 'true']),
# enumerates the options based on the artifacts that are defined below
OptEnum.new('APPCATEGORY', [false, 'Category of applications to gather from', 'All', APPLICATION_ARRAY.map {|x| x[:category]}.uniq.unshift('All')]),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
OptEnum.new('APPCATEGORY', [false, 'Category of applications to gather from', 'All', APPLICATION_ARRAY.map {|x| x[:category]}.uniq.unshift('All')]),
OptEnum.new('APP_CATEGORY', [false, 'Category of applications to gather from', 'All', APPLICATION_ARRAY.map {|x| x[:category]}.uniq.unshift('All')]),

and refactor references


# Check to see if the artifact exists on the remote system.
def location(profile, opts = {})

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete empty line for consistency

artifact_child[:xml_search].each do |xml_split|
xml_split[:xml].each do |xml_string|
xml_file.xpath("#{xml_string}").each do |xml_match|
vprint_status("#{xml_split[:extraction_description]}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be a little hard to debug this module if you're just printing the variable data with no context. For this (and most other) vprint_*() calls, try to add some context to what you're printing

end

sql_credential_path = store_loot("#{artifact}#{cred}", "", session, "#{database_string}", local_loc) #saves neatened up database file
print_status "File with credentials saved: #{sql_credential_path}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
print_status "File with credentials saved: #{sql_credential_path}"
print_status "File with credentials saved: #{sql_credential_path}"


child_json_query.each do |split|
children = eval("json_parse#{parent_json_query}")
children.each {|child_node|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the same block styling for inline blocks:

children.each do |child_node|
  .......
end

# filter based on options
if (category != datastore['APPCATEGORY'] && datastore['APPCATEGORY'] != 'All') || (application != datastore['APPLICATION'] && datastore['APPLICATION'] != 'All') || (file_type != datastore['ARTIFACTS'] && datastore['ARTIFACTS'] != 'All')
# doesn't match search criteria, skip this artifact
next
Copy link
Contributor

@cbrnrd cbrnrd Jun 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
next
next if (category != datastore['APPCATEGORY'] && datastore['APPCATEGORY'] != 'All') || (application != datastore['APPLICATION'] && datastore['APPLICATION'] != 'All') || (file_type != datastore['ARTIFACTS'] && datastore['ARTIFACTS'] != 'All')

and delete end statement on the next line


if credential_type == 'sqlite'
extract_sqlite(saving_path, artifact_child, artifact, local_loc)
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These can all be compressed into one line each:

extract_xml(saving_path, artifact_child, artifact, local_loc) if credential_type == 'xml'

extract_json(saving_path, artifact_child, artifact, local_loc) if credential_type == 'json'

extract_regex(saving_path, artifact_child, artifact, local_loc) if credential_type == 'text'

extract_sqlite(saving_path, artifact_child, artifact, local_loc) if credential_type == 'sqlite'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or you could turn it into a case/switch, up to you.

1 Game:
Xfire

13 Browser:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
13 Browser:
13 Browsers:

@wvu
Copy link
Contributor

wvu commented Jun 20, 2019

@cbrnrd: I'm not sure @danhallsworth1 is coming back, so any review you leave will be more work for me. :-)

@wvu
Copy link
Contributor

wvu commented Jun 20, 2019

@cbrnrd: How about I close this PR and open up a new one you can PR to? Then I can address the changes directly.

@h00die
Copy link
Contributor

h00die commented Jun 20, 2019

My only real issue with this module is that it's a huge pile of modules. In theory it would be broken down into parts, each functional item, then packrat could be a post script that runs them all.
This makes it easier to maintain, test, and doesn't lose functionality

@h00die
Copy link
Contributor

h00die commented Jun 20, 2019

Not to mention, and I didn't check, there's possible overlap in code and functionality between this and other modules that already exist

@wvu
Copy link
Contributor

wvu commented Jun 20, 2019

I have already written code for this PR, and I am unable to contribute further at the moment. Someone else will have to. Eh, opening a new PR. Let's find a way to carry this across the finish line.

@wvu
Copy link
Contributor

wvu commented Jun 21, 2019

See me over in #11998. Thanks.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants