Skip to content

fix(api): prevent path traversal in /tasks/file endpoint (CWE-22) - #230

Open
sebastionoss wants to merge 1 commit into
zai-org:mainfrom
sebastionoss:fix/cwe22-tasks-file-83c5
Open

fix(api): prevent path traversal in /tasks/file endpoint (CWE-22)#230
sebastionoss wants to merge 1 commit into
zai-org:mainfrom
sebastionoss:fix/cwe22-tasks-file-83c5

Conversation

@sebastionoss

Copy link
Copy Markdown

Summary

The GET /api/v1/tasks/file endpoint in apps/backend/app/api/tasks.py accepts a user-supplied path query parameter and passes it directly to pathlib.Path(...) before reading the file contents. There is no validation that the resolved path stays inside the intended output directory, and the endpoint has no authentication (the router is mounted without any Depends(...) gate and apps/backend/app/main.py only registers CORSMiddleware). Any network-reachable client can read arbitrary files that the backend process can access — including /etc/passwd, application config, .env files with API keys, private SSH keys, etc.

  • CWE: CWE-22 — Improper Limitation of a Pathname to a Restricted Directory
  • Affected file/function: apps/backend/app/api/tasks.py, read_file() (route GET /api/v1/tasks/file)
  • Severity: High — unauthenticated arbitrary file read from the backend host
  • Preconditions: network reachability to the backend (the default deployment via the provided Dockerfile exposes the API without auth)

Data flow

path (query string, attacker controlled) → Path(path)file_path.exists() / file_path.read_text() / FileResponse(file_path). No canonicalisation, no base-directory check, no allow-list.

Fix

Canonicalise the requested path with Path(...).resolve() and verify — via relative_to(base_dir) — that it is contained within settings.OUTPUT_DIR (also resolved). Anything outside returns 403; unresolvable paths return 400. This blocks ../ traversal, absolute paths (/etc/passwd), and symlink escapes (because resolve() follows symlinks before the containment check).

base_dir = Path(settings.OUTPUT_DIR).resolve()
try:
    file_path = Path(path).resolve(strict=False)
except (OSError, RuntimeError):
    raise HTTPException(status_code=400, detail="Invalid file path")
try:
    file_path.relative_to(base_dir)
except ValueError:
    raise HTTPException(status_code=403, detail="Access to the requested path is not allowed")

The change is 16 lines, localised to the one vulnerable handler, and preserves the legitimate use case (reading task outputs written under OUTPUT_DIR).

Proof of Concept

Against a local run of the backend (uvicorn app.main:app from apps/backend, default OUTPUT_DIR):

# Before the fix — reads an arbitrary system file:
curl -s "http://127.0.0.1:8000/api/v1/tasks/file?path=/etc/passwd" | head
# root:x:0:0:root:/root:/bin/bash
# ...

# Traversal from a relative path also works:
curl -s "http://127.0.0.1:8000/api/v1/tasks/file?path=../../../../etc/passwd"

# After the fix — both return 403:
curl -i "http://127.0.0.1:8000/api/v1/tasks/file?path=/etc/passwd"
# HTTP/1.1 403 Forbidden
# {"detail":"Access to the requested path is not allowed"}

# Legitimate access to a task output file still works:
curl -i "http://127.0.0.1:8000/api/v1/tasks/file?path=$(pwd)/outputs/<task_id>/result.md"
# HTTP/1.1 200 OK

Testing

  • Verified the vulnerable path resolves to real system files before the patch.
  • Verified /etc/passwd, ../../etc/passwd, and other out-of-base inputs return 403 after the patch.
  • Verified a path constructed under the resolved OUTPUT_DIR still returns 200 and file contents.
  • Verified symlink escape (ln -s /etc/passwd $OUTPUT_DIR/link; request ?path=$OUTPUT_DIR/link) is blocked because resolve() follows the symlink prior to the relative_to check.

Adversarial review

Before submitting we tried to disprove this. We checked whether the router or the FastAPI app applied any auth dependency that would make the endpoint unreachable to anonymous clients — apps/backend/app/main.py mounts tasks_router with only a prefix and CORSMiddleware, and APIRouter(prefix="/tasks", ...) in tasks.py has no dependencies=[Depends(...)]. We also considered whether OUTPUT_DIR being an absolute path would defeat the check — it doesn't, because both sides are resolve()d before comparison. Finally we considered whether the endpoint might already be gated at deployment (reverse proxy, network policy); the shipped Dockerfile exposes the API port directly with no auth layer, so the default deployment is exploitable.


Discovered by the Sebastion AI GitHub App.

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.

1 participant