Skip to content

chore: create a section on session hijack #641

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

SamMorrowDrums
Copy link

Adds a section in the security best practices document on potential for session hijacking, and mitigations

Motivation and Context

With a horizontally scaled http server, there is a potential for session hijacking with incorrect implementation.

How Has This Been Tested?

Documentation only.

Breaking Changes

None

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@localden localden added security documentation Improvements or additions to documentation enhancement New feature or request labels Jun 3, 2025
@connor4312
Copy link
Contributor

It may be worth adding a bullet point under no.1 in session management like:

  • The client MUST treat the session ID as a credential and handle it in a secure manner.

(I'm not sure if MUST/SHOULD verbiage is compatible with the fuzzy requirement of treating it in a 'secure manner')

MCP servers **SHOULD** bind session keys to user-specific information.
When storing or transmitting session-related data (e.g., in a queue), combine the session ID with information unique to the authorized user, such as their internal user ID. Use a key format like `<user_id>:<session_id>`. This ensures that even if an attacker guesses a session ID, they cannot impersonate another user as the user ID is derived from the user token and not provided by the client.

MCP servers can optionally leverage additional unique identifiers.
Copy link
Contributor

@connor4312 connor4312 Jun 4, 2025

Choose a reason for hiding this comment

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

IP address binding may not generally best practice as the IP address from users on or tethering to mobile devices is apt to change frequently when roaming.

Copy link
Author

Choose a reason for hiding this comment

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

It's admittedly hard to find good alternatives if the user isn't logged in. I did hesitate for this, as it does cause false negatives. I just wanted to add that there can be other data which can work. I'm happy to drop it if it doesn't add much.

Also, according to the spec, if you try to use a session ID and the session is no longer valid and your request is rejected, you get foreced to re-initialize. That can break the state on a stateful server though, so not exactly ideal, but for most scenarios it would just continue to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants