AOSSIE-Org / AOSSIE-Org/EduAid

Security: FileProcessor.process_file uses client-controlled filename as on-disk path

Offen
#676 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Vorherrschende Sprache
JavaScript
Sterne
171
Forks
425
PR-Merge-Kennzahlen
Keine gemergten PRs in 30 T.

Beschreibung

Flagged by CodeRabbit review on PR #675 (the logic is carried over verbatim from the original `backend/Generator/main.py`, so this predates the refactor).

## Problem
`backend/Generator/utilities.py` -> `FileProcessor.process_file()` saves the uploaded file using `file.filename` directly:

```python
file_path = os.path.join(self.upload_folder, file.filename)
file.save(file_path)
```

A crafted filename such as `../../app.py` (or absolute paths / null bytes depending on the WSGI layer) escapes `uploads/`. Werkzeug's `FileStorage.save` does sanitize some cases, but relying on framework-level protection is fragile, and `os.path.join` itself will happily escape when given an absolute path.

## Suggested fix
Keep the extension, discard the client-controlled path components:

```python
import uuid

def process_file(self, file):
ext = os.path.splitext(file.filename)[1].lower()
safe_name = f"{uuid.uuid4().hex}{ext}"
file_path = os.path.join(self.upload_folder, safe_name)
...
```

(Or `os.path.basename(file.filename)` at minimum, plus an allowlist of `.txt/.pdf/.docx`.)

## Extra notes
- The subsequent `.endswith('.txt'/'.pdf'/'.docx')` dispatch already implies an allowlist; enforcing it before saving closes the hole fully.
- Happy to submit this as a PR once triaged.

Beitragsleitfaden

Für dieses Repository ist kein Beitragsleitfaden indexiert

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.