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

Manually set `needs_cleanup` for exploits that need it #12158

Merged
merged 3 commits into from Aug 2, 2019

Conversation

@acammack-r7
Copy link
Contributor

commented Aug 2, 2019

The needs_cleanup flag needs to be set per-module when an exploit needs an interactive session to clean up. Some FileDropper exploits need additional cleanup to what the mixin provides, but since all FileDroppers already mark themselves as needing cleanup those are not covered here. A few of these could potentially be refactored to use the original exploitation method to clean up or to compile the list of files/commands to clean up ahead of time, but that is out of the scope of this fix.

Also cleaned up some glaring white space issues I saw while auditing these modules.

As far as the future goes, after auditing all the in-module uses of on_new_session, I think that any meaningful cleanup capability in pingback payloads will need to be accompanied by an overhaul of how we think about cleanup as a whole. Also, it will likely be useful in the future to separate out "I want to do this on a new session" (several modules not touched in this PR) from "I need to do this on the target to keep it stable" (some of these modules) and from "I need to do this on the target to maintain good opsec" (most of these).

Verification

  • ./msfconsole
  • use the modules in this PR
  • Verify show payloads does not show pingback payloads

acammack-r7 added some commits Aug 2, 2019

Set `needs_cleanup` flag for exploits that need it
The `needs_cleanup` flag needs to be set per-module when an exploit
needs an interactive session to clean up. Some `FileDropper` exploits
need additional cleanup to what the mixin provides, but since all
`FileDropper`s already mark themselves as needing cleanup those are not
covered here. A few of these could potentially be refactored to use the
original exploitation method to clean up or to compile the list of
files/commands to clean up ahead of time, but that is out of the scope
of this fix.
@wvu-r7

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2019

I checked the cases where you didn't add needs_cleanup. I think you got them all. Thanks for doing this.

@wvu-r7

wvu-r7 approved these changes Aug 2, 2019

@wvu-r7 wvu-r7 added bug module labels Aug 2, 2019

@wvu-r7 wvu-r7 self-assigned this Aug 2, 2019

@wvu-r7 wvu-r7 merged commit e11de69 into rapid7:master Aug 2, 2019

3 checks passed

Metasploit Automation - Sanity Test Execution Successfully completed all tests.
Details
Metasploit Automation - Test Execution Successfully completed all tests.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

wvu-r7 added a commit that referenced this pull request Aug 2, 2019

msjenkins-r7 added a commit that referenced this pull request Aug 2, 2019

@wvu-r7

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2019

Release Notes

Pingback payloads are now blacklisted for exploits utilizing the on_new_session callback to perform post-exploitation cleanup. Previously, only exploits including the FileDropper mixin, which also uses on_new_session, were blacklisted.

@jmartin-r7 jmartin-r7 added the msf5 label Aug 2, 2019

jmartin-r7 added a commit that referenced this pull request Aug 2, 2019

@tdoan-r7 tdoan-r7 added the rn-fix label Aug 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.