Skip to content

fix(pipelines): prevent arbitrary file read and path traversal in /files endpoint - #11247

Open
AdvaitVarhade wants to merge 1 commit into
PaddlePaddle:developfrom
AdvaitVarhade:fix/rest-api-files-path-traversal
Open

fix(pipelines): prevent arbitrary file read and path traversal in /files endpoint#11247
AdvaitVarhade wants to merge 1 commit into
PaddlePaddle:developfrom
AdvaitVarhade:fix/rest-api-files-path-traversal

Conversation

@AdvaitVarhade

Copy link
Copy Markdown

Summary of Changes

This pull request resolves a path traversal and arbitrary file read vulnerability in the pipelines REST API /files endpoint (slm/pipelines/rest_api/controller/file_upload.py).

Problem

In the previous implementation of download_file, os.path.join(FILE_PARSE_PATH, file_name) was used directly without path sanitization or boundary verification. Supplying absolute paths (e.g., /etc/passwd or C:\Windows\win.ini) or directory traversal sequences (e.g., ../../file) caused os.path.join to discard FILE_PARSE_PATH or traverse outside the intended directory.

Solution

  1. Input Sanitization: Strip directory components using os.path.basename(file_name).
  2. Boundary Validation: Check that abs_target starts with abs_base + os.sep (where abs_base = os.path.abspath(FILE_PARSE_PATH)).
  3. Invalid Request Handling: If file_name contains path traversal attempts or resolves outside FILE_PARSE_PATH, raise HTTPException(status_code=400, detail="Invalid file name").
  4. Regular File Verification: Ensure only existing regular files (os.path.isfile) are returned via FileResponse.
  5. Unit Testing: Added unit tests in slm/pipelines/tests/test_file_upload.py covering valid file downloads, default filename behavior, absolute path traversal attempts, relative directory traversal, and non-existent files.

We welcome feedback from the maintainers on this implementation!

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


AdvaitVarhade seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

2 participants