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

Fix username/password generation in case both PASSWORD_SPRAY and USER_AS_PASS are enabled #19550

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Mathiou04
Copy link

@Mathiou04 Mathiou04 commented Oct 10, 2024

Fixing bug #19525

I think this is the minimal code for fixing this bug.

But looking at the each_filtered method, I think I found a few more bugs along the way.
I didn't want to go into this to make this review easier but I think it would be worth spending a bit more time on this method to:

  • add test cases to show the suspected bugs & fix them
  • take some time to refactor this code a bit (it looks like the each_unfiltered_password_first, introduced for the PASSWORD_SPRAY option, is a quick adaptation of the each_unfiltered_username_first, maybe with a few mistakes)

This is my first PR for the metasploit project and I've looked at the basecode for the first time today, so sorry in advance if I missed something in the process 😓

…e PASSWORD_SPRAY and USER_AS_PASS are both enabled

Added minimal code to fix the issue, extracting the code to generate username:username credentials in the PASSWORD_SPRAY case
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants