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

Zeitwerk msf folder #14629

Merged
merged 13 commits into from Jan 22, 2021
Merged

Zeitwerk msf folder #14629

merged 13 commits into from Jan 22, 2021

Conversation

dwelch-r7
Copy link
Contributor

@dwelch-r7 dwelch-r7 commented Jan 18, 2021

Continuation of the zeitwerk initiative we have going on, following on from this pr #14202 and #14584

In this PR we have zeitwerked the lib/msf/ folder

Verification steps

  • All framework tests pass
  • Metasploit Pro tests pass

On Linux

  • Console boots up
  • search for and use a module
  • Sucessfully run a module
  • Generate a payload with msfvenom
  • Gain a meterpreter session

On Osx

  • Console boots up
  • search for and use a module
  • Sucessfully run a module
  • Generate a payload with msfvenom
  • Gain a meterpreter session

On Windows

  • Console boots up
  • search for and use a module
  • Sucessfully run a module
  • Generate a payload with msfvenom
  • Gain a meterpreter session

@dwelch-r7 dwelch-r7 marked this pull request as ready for review January 20, 2021 11:56
@@ -1,5 +1,3 @@
# -*- coding: binary -*-
Copy link
Contributor

Choose a reason for hiding this comment

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

Sanity question, is this needed still?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it shouldn't be, it only seems to affect any strings defined in the file and there aren't any 😄
weren't any before so it was probably a copy/paste job

Copy link
Contributor

Choose a reason for hiding this comment

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

That's my understanding too, just calling it out for visibility - as there seemed to be a couple of historical bulk changes to add this to every file:
d656e31

I imagine there was a solid reason for it that, but the commit doesn't have any further details. It could've just been the easiest to automate, adding any missing binary comment regardless of the file's content

#
# The core library provides all of the means by which to interact
# with the framework insofar as manipulating encoders, nops,
# payloads, exploits, auxiliary, and sessions.
Copy link
Contributor

Choose a reason for hiding this comment

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

These comments have been useful in the past, does it make sense to move it elsewhere? 🤔

Since we plan on deleting this file - I'm not sure where it would live within our code. I've seen some projects have readme.md files in directories of interest, which act as a bread crumb trail as to what the folders are meant to do at the conceptual level. I don't believe having it in the wiki is the right solution either, as the documentation being colocated with the folder will be naturally found when code spelunking

This comment applies to the previously deleted base.rb comments too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Personally I don't see a tonne of value in these comments but I'd definitely be down for the readme idea if we can keep it up to date

@adfoster-r7
Copy link
Contributor

Trying to restart msfdb:

bundle exec ruby ./msfdb restart
Traceback (most recent call last):
./msfdb:30:in `<main>': uninitialized constant Msf::Config (NameError)
Did you mean?  RbConfig

@dwelch-r7
Copy link
Contributor Author

Trying to restart msfdb:

bundle exec ruby ./msfdb restart
Traceback (most recent call last):
./msfdb:30:in `<main>': uninitialized constant Msf::Config (NameError)
Did you mean?  RbConfig

Hmmm I thought I fixed that, let me take a look

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tools/exploit/exe2vba.rb: uninitialized constant Msf::Util::EXE (NameError)
2 participants