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

Don't close sockets that we're using for sessions #19056

Conversation

dwelch-r7
Copy link
Contributor

@dwelch-r7 dwelch-r7 commented Apr 5, 2024

Spotted an issue where when targeting a single host with multiple user/pass combinations that if a session was created then the next login attempt would close the socket being used by the newly created session. I've fixed this by explicitly setting the socket to nil in the login scanner.
There were also some issues with the error handling where we were trying to access a clients socket incorrectly so I've addressed that in here too by passing the socket in as part of the result object so all error handling is the same across implementations.

Verification Steps

  • boot up msfconsole
  • create a set of user/pass combos
  • use each of the login modules with the set of credentials
  • ensure a session is generated for each of mysql_login, postgresql_login, mssql_login and smb_login
  • interact with the session(s) to make sure they remain valid after more attempted logins

@dwelch-r7 dwelch-r7 added the rn-fix release notes fix label Apr 5, 2024
@dwelch-r7 dwelch-r7 marked this pull request as ready for review April 5, 2024 10:42
@dwelch-r7 dwelch-r7 force-pushed the keep-sockets-open-when-used-for-a-session branch from ccc9417 to ac0a138 Compare April 5, 2024 12:10
@adfoster-r7
Copy link
Contributor

Needs a rebase 👍

@dwelch-r7 dwelch-r7 force-pushed the keep-sockets-open-when-used-for-a-session branch from ac0a138 to 87b84b0 Compare April 5, 2024 13:33
@cgranleese-r7 cgranleese-r7 self-assigned this Apr 8, 2024
@cgranleese-r7
Copy link
Contributor

Tested against:

  • postgres:16.1
  • mcr.microsoft.com/mssql/server:2022-preview-ubuntu-22.04
  • mariadb:11.2.2
  • Windows 10

Everything seemed to be working as expected 👍

Before

image

After

image

@cgranleese-r7
Copy link
Contributor

Have a question, in the description you mention:

I've fixed this by explicitly setting the socket to nil in the login scanner.

But it seems that was only done in the login scanner for MySQL and was already present in SMB. Just wondering if the description is outdated or was it not required for MSSQL or Postgres?

Copy link
Contributor

@cgranleese-r7 cgranleese-r7 left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@cgranleese-r7 cgranleese-r7 merged commit 951da5b into rapid7:master Apr 8, 2024
45 checks passed
@cgranleese-r7
Copy link
Contributor

Release Notes

This PR fixes an issue were the socket would be closed if targeting a single host with multiple user_file/pass_file module option combinations. This was caused when a session was successfully opened but then the next login attempt would close the socket being used by the newly created session.

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

Successfully merging this pull request may close these issues.

3 participants