Skip to content

security(utilities): harden path traversal validation in safe_extract#12347

Merged
StpMax merged 2 commits into
mindsdb:releases/26.1.0from
SyedaAnshrahGillani:fix/path-traversal-validation
Apr 17, 2026
Merged

security(utilities): harden path traversal validation in safe_extract#12347
StpMax merged 2 commits into
mindsdb:releases/26.1.0from
SyedaAnshrahGillani:fix/path-traversal-validation

Conversation

@SyedaAnshrahGillani
Copy link
Copy Markdown
Contributor

This PR fixes a security vulnerability in the path traversal validation logic within mindsdb/utilities/fs.py.

The previous implementation relied on os.path.commonprefix, which performs a character-by-character string comparison rather than a path-component-aware check. This made the validation susceptible to bypasses where a malicious target path shares a prefix with the intended directory but resides outside of it (e.g., /app/data incorrectly validating /app/data-secret).

Changes:

  • Replaced the flawed os.path.commonprefix check with a robust os.path.relpath validation in __is_within_directory.
  • Added more descriptive error messages to safe_extract to provide the specific member name when a traversal attempt is
    detected, aiding in security auditing and debugging.

Fixes # (if there is a related issue, otherwise leave blank)

Type of change

  • 🐛 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 not to work as expected)
  • 📄 This change requires a documentation update

Verification Process

To ensure the changes are working as expected:

- Test Location: mindsdb/utilities/fs.py
- Verification Steps:
   1. Verified the logic using a standalone test script to check edge cases:
      - /app/data vs /app/data-secret (Now correctly returns False)
      - /app/data vs /app/data/file.txt (Correctly returns True)
      - /app/data vs /app/other/file.txt (Correctly returns False)
   2. Confirmed that safe_extract correctly raises an exception with the member name when a .. traversal is attempted in a
      mock tarfile.

Checklist:

  • My code follows the style guidelines(PEP 8) of MindsDB.
  • I have appropriately commented on my code, especially in complex areas.
  • Necessary documentation updates are either made or tracked in issues.
  • Relevant unit and integration tests are updated or added.

Replaced the flawed os.path.commonprefix check with a robust
os.path.relpath validation in __is_within_directory. The previous
implementation was susceptible to bypasses where a target path
started with the same prefix as the directory (e.g., /app/data
and /app/data-secret).

Added more descriptive error messages to safe_extract to provide
better context when a traversal attempt is detected.
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 31, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@Syeda-Anshrah-Gillani
Copy link
Copy Markdown
Contributor

I have read the CLA Document and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request Mar 31, 2026
@StpMax StpMax changed the base branch from main to releases/26.1.0 April 17, 2026 08:49
@StpMax StpMax merged commit bc554ec into mindsdb:releases/26.1.0 Apr 17, 2026
11 of 13 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 17, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants