From 21eeb513174dd89fa50ef57eed01a8731b9dea09 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Wed, 25 Mar 2026 09:00:45 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[HIGH]=20Fi?= =?UTF-8?q?x=20OOM=20DoS=20and=20TOCTOU=20in=20JSON=20file=20adapters?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaced vulnerable `path.stat().st_size` checks with secure bounded reads (`f.read(limit + 1)`) and added `path.is_file()` validation across all file-backed JSON adapters to prevent out-of-memory denial-of-service via device files like `/dev/zero`. Co-authored-by: ivangegovdve-sudo <225339531+ivangegovdve-sudo@users.noreply.github.com> --- .jules/sentinel.md | 5 +++++ .../adapters/checkpoint_store.py | 8 +++++--- .../adapters/json_file_practice_repository.py | 14 ++++++++------ .../adapters/json_file_progress_repository.py | 14 ++++++++------ .../adapters/json_file_progress_snapshot_store.py | 14 ++++++++------ 5 files changed, 34 insertions(+), 21 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 3ae152f..f78f114 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -2,3 +2,8 @@ **Vulnerability:** Unbounded memory consumption DoS risk due to JSON file parsing without size limits in file-backed repositories and stores. **Learning:** `json.loads(file.read_text())` reads the entire file content into memory. This can lead to out-of-memory errors or denial-of-service if an attacker or misconfiguration provides an excessively large file. The application relies heavily on file-backed adapters (e.g., `JsonFilePracticeRepository`, `JsonFileProgressSnapshotStore`, `CheckpointStore`). **Prevention:** Implement a strict file size limit check using `path.stat().st_size` (e.g., standardized at 10MB or `10 * 1024 * 1024` bytes) before reading the file content into memory in all file-based adapters. + +## 2024-02-23 - [Device File Size Read Bypass / TOCTOU] +**Vulnerability:** `path.stat().st_size` checks before `path.read_text()` are vulnerable to TOCTOU and bypass via device files (like `/dev/zero`), which report size 0 but produce infinite data. +**Learning:** To prevent unbounded memory consumption and DoS, verifying `path.is_file()` first is crucial. However, the most secure approach combines `path.is_file()` with a bounded read pattern (`f.read(limit + 1)`) instead of relying solely on `st_size` beforehand. +**Prevention:** Ensure file adapters always use `path.is_file()` to reject device files and perform bounded reads (`content = f.read(LIMIT + 1); if len(content) > LIMIT: raise ValueError(...)`) instead of relying on `path.stat().st_size`. diff --git a/src/python_learning_orchestrated/adapters/checkpoint_store.py b/src/python_learning_orchestrated/adapters/checkpoint_store.py index 0d9c5d5..4b733ba 100644 --- a/src/python_learning_orchestrated/adapters/checkpoint_store.py +++ b/src/python_learning_orchestrated/adapters/checkpoint_store.py @@ -231,11 +231,13 @@ def _to_int(value: object, default: int) -> int: def _read_json(path: Path) -> dict[str, object]: - if not path.exists(): + if not path.is_file(): return {} - if path.stat().st_size > 10 * 1024 * 1024: + with path.open("r", encoding="utf-8") as f: + content = f.read(10 * 1024 * 1024 + 1) + if len(content) > 10 * 1024 * 1024: raise ValueError(f"Checkpoint file {path} exceeds 10MB size limit") - parsed = json.loads(path.read_text(encoding="utf-8")) + parsed = json.loads(content) return parsed if isinstance(parsed, dict) else {} diff --git a/src/python_learning_orchestrated/adapters/json_file_practice_repository.py b/src/python_learning_orchestrated/adapters/json_file_practice_repository.py index 15da61b..c07db7c 100644 --- a/src/python_learning_orchestrated/adapters/json_file_practice_repository.py +++ b/src/python_learning_orchestrated/adapters/json_file_practice_repository.py @@ -103,15 +103,17 @@ def record_attempts(self, attempts: list[Attempt]) -> None: self._save_storage(storage) def _load_storage(self) -> dict[str, object]: - if not self._file_path.exists(): + if not self._file_path.is_file(): return {"items": [], "attempts": []} - if self._file_path.stat().st_size > 10 * 1024 * 1024: - raise ValueError( - f"Practice repository file {self._file_path} exceeds 10MB size limit" - ) try: - content = self._file_path.read_text(encoding="utf-8") + with self._file_path.open("r", encoding="utf-8") as f: + content = f.read(10 * 1024 * 1024 + 1) + if len(content) > 10 * 1024 * 1024: + raise ValueError( + f"Practice repository file {self._file_path} " + "exceeds 10MB size limit" + ) if not content.strip(): return {"items": [], "attempts": []} parsed = json.loads(content) diff --git a/src/python_learning_orchestrated/adapters/json_file_progress_repository.py b/src/python_learning_orchestrated/adapters/json_file_progress_repository.py index 7c956af..a5286d3 100644 --- a/src/python_learning_orchestrated/adapters/json_file_progress_repository.py +++ b/src/python_learning_orchestrated/adapters/json_file_progress_repository.py @@ -40,15 +40,17 @@ def reset_progress(self, user_id: str) -> None: def _load_storage(self) -> dict[str, LessonProgress]: """Load all persisted progress payloads.""" - if not self._file_path.exists(): + if not self._file_path.is_file(): return {} - if self._file_path.stat().st_size > 10 * 1024 * 1024: - raise ValueError( - f"Progress repository file {self._file_path} exceeds 10MB size limit" - ) try: - content = self._file_path.read_text(encoding="utf-8") + with self._file_path.open("r", encoding="utf-8") as f: + content = f.read(10 * 1024 * 1024 + 1) + if len(content) > 10 * 1024 * 1024: + raise ValueError( + f"Progress repository file {self._file_path} " + "exceeds 10MB size limit" + ) if not content.strip(): return {} diff --git a/src/python_learning_orchestrated/adapters/json_file_progress_snapshot_store.py b/src/python_learning_orchestrated/adapters/json_file_progress_snapshot_store.py index ac34e1c..b1bb2a2 100644 --- a/src/python_learning_orchestrated/adapters/json_file_progress_snapshot_store.py +++ b/src/python_learning_orchestrated/adapters/json_file_progress_snapshot_store.py @@ -34,14 +34,16 @@ def save(self, snapshot: ProgressSnapshot) -> None: self._save_payload(progress_snapshot_to_payload(snapshot)) def _load_payload(self) -> dict[str, object]: - if not self._file_path.exists(): + if not self._file_path.is_file(): return {} - if self._file_path.stat().st_size > 10 * 1024 * 1024: - raise ValueError( - f"Progress snapshot file {self._file_path} exceeds 10MB size limit" - ) try: - parsed = json.loads(self._file_path.read_text(encoding="utf-8")) + with self._file_path.open("r", encoding="utf-8") as f: + content = f.read(10 * 1024 * 1024 + 1) + if len(content) > 10 * 1024 * 1024: + raise ValueError( + f"Progress snapshot file {self._file_path} exceeds 10MB size limit" + ) + parsed = json.loads(content) except (OSError, json.JSONDecodeError): return {} return parsed if isinstance(parsed, dict) else {}