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

Race condition in RPC session API #19013

Open
FenixH opened this issue Mar 25, 2024 · 2 comments
Open

Race condition in RPC session API #19013

FenixH opened this issue Mar 25, 2024 · 2 comments
Labels
bug Stale Marks an issue as stale, to be closed if no action is taken

Comments

@FenixH
Copy link
Contributor

FenixH commented Mar 25, 2024

Sessions are registered at the framework here:

framework.sessions.register(session)

This is an issue as not all initital logic for the session has been run. For example, bootstrap functions by the session handler, auto-run scripts and custom event handlers. The issue is that the API server lists sessions as soon as they are registered: https://github.com/rapid7/metasploit-framework/blob/master/lib/msf/core/rpc/v10/rpc_session.rb#L31

Apart from the obvious issue of a user/script running a command in a session based on what the API server lists (before the session is ready) there are also race conditions possible by simply calling the session get parameters - which are called by the rpc_list command.

For example, session.tunnel_peer can easily be set to 127.0.0.1 for an interactive session if it is called too early:


This causes a number of issues, for example, related to routing.

I am a bit uncertain on the best-practice method to solve this issue. I present two example alternatives:

  • One way would be to move
    framework.sessions.register(session)
    further down, perhaps just before session_waiter_event.notify(session). I am unsure regarding the consequences to other parts of the framework if this is done though.
  • Another way is to allow the rpc session API to listen to session events (framework.events.add_session_subscriber(self)) and have its own internal list of "ready" sessions.

I am happy to provide a patch for either solution, should you deem it useful.

@FenixH FenixH added the bug label Mar 25, 2024
@nrathaus
Copy link
Contributor

@FenixH can you provide a PR for this bug? also can you give some steps to reproduce? I think I am experiencing similar issues when I call Metasploit via pymetasploit3 - I artificially added a wait (sleep) before calling additional calls to the session that was created

Copy link

Hi!

This issue has been left open with no activity for a while now.

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 30 days since the last update here.
If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!

As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request.

@github-actions github-actions bot added the Stale Marks an issue as stale, to be closed if no action is taken label May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Stale Marks an issue as stale, to be closed if no action is taken
Projects
Status: No status
Development

No branches or pull requests

2 participants