Skip to content

Conversation

@jope-bm
Copy link
Contributor

@jope-bm jope-bm commented Jul 10, 2025


title: Path Traversal Security Vulnerability Fix
type: note
permalink: security/path-traversal-security-vulnerability-fix
tags:

  • '["security"'
  • '"vulnerability"'
  • '"path-traversal"'
  • '"mcp"'
  • '"fix"]'

Path Traversal Security Vulnerability Fix

Overview

Fixed a critical path traversal security vulnerability in Basic Memory's MCP tools that could allow malicious file system access outside of intended project directories.

Vulnerability Details

  • Type: Path Traversal (CWE-22)
  • Severity: High
  • Impact: Potential unauthorized file system access
  • Affected Components: MCP tools that handle file paths
  • Discovery: Internal security review

Root Cause

Several MCP tools were accepting user-provided file paths without proper validation, allowing potential directory traversal attacks using sequences like ../ to access files outside the intended project scope.

Affected Tools

The following MCP tools were vulnerable:

  • read_content - Could read arbitrary files on the system
  • write_note - Could write files outside project directories
  • read_note - Could modify files outside project scope
  • move_note - Could move files to/from unauthorized locations

Fix Implementation

  • Protection Function: Added protect_path() function in src/basic_memory/mcp/utils.py
  • Validation Logic: Ensures all file paths are within the current project directory
  • Normalization: Uses os.path.normpath() to resolve path traversal sequences
  • Security Check: Validates that resolved paths start with the project root
  • Error Handling: Raises ValueError for invalid paths with descriptive messages

Code Changes

  • Added path protection utility function
  • Integrated protection into all vulnerable MCP tools
  • Added comprehensive test coverage for path traversal scenarios
  • Updated error messages to be security-aware

Testing

  • Created test suite covering various path traversal attack vectors
  • Verified protection against ../ sequences
  • Tested edge cases and boundary conditions
  • Confirmed legitimate file operations still work correctly
  • Tested mcp tools to create new note and read note using Claude Code.

Security Impact

  • Before: Attackers could potentially access any file on the system
  • After: File access restricted to project directories only
  • Defense: Proactive validation prevents path traversal attacks

Related Information

  • Security Classification: Defensive security measure
  • Review: Internal security assessment completed

jope-bm added 2 commits July 10, 2025 16:09
Signed-off-by: Joe P <joe@basicmemory.com>
Signed-off-by: Joe P <joe@basicmemory.com>
@jope-bm jope-bm force-pushed the fix/path-traversal-security-vulnerability-mcp branch from 43f33f8 to 31b2f39 Compare July 10, 2025 22:10
Copy link
Member

@phernandez phernandez left a comment

Choose a reason for hiding this comment

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

Good Lord, man. That is a lot of tests.

@phernandez phernandez changed the title Fix path traversal security vulnerability in mcp tools fix: path traversal security vulnerability in mcp tools Jul 10, 2025
@jope-bm
Copy link
Contributor Author

jope-bm commented Jul 10, 2025

Good Lord, man. That is a lot of tests.

Yeah claude went kind of nuts and didn't mock the behavior of validate_project_path() inside the tools, and so it validated the behavior of validate_project_path() even though we have separate unit tests for that. I can have claude mock validate_project_path so the tool tests aren't so much overkill.

@phernandez phernandez added this to the v0.14.3 milestone Jul 10, 2025
@jope-bm jope-bm merged commit 24a1d61 into main Jul 15, 2025
9 of 10 checks passed
@jope-bm jope-bm deleted the fix/path-traversal-security-vulnerability-mcp branch July 15, 2025 15:05
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