AOSSIE-Org / AOSSIE-Org/EduAid

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

Đang mở
#676 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
JavaScript
Star
171
Fork
425
Chỉ số merge pull request
Không có pull request nào được merge trong 30 ngày

Mô tả

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.

Hướng dẫn đóng góp

Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.