AOSSIE-Org / AOSSIE-Org/EduAid

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

Ouverte
#676 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
Langage dominant
JavaScript
Étoiles
171
Forks
425
Métriques de merge des PR
Aucune PR mergée en 30 j

Description

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.

Guide de contribution

Aucun guide de contribution indexé pour ce dépôt

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.