From a6a8668c95aa00ae8b1f95b6fc4365652b4513f7 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Tue, 14 Jul 2026 21:23:48 +0000 Subject: [PATCH 01/11] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[HIGH?= =?UTF-8?q?]=20Fix=20URL=20=EC=9D=B8=EC=BD=94=EB=94=A9=EB=90=9C=20?= =?UTF-8?q?=EA=B2=BD=EB=A1=9C=20=ED=83=90=EC=83=89=20=EC=B7=A8=EC=95=BD?= =?UTF-8?q?=EC=A0=90=20(Path=20Traversal)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .jules/sentinel.md | 5 +++++ CHANGELOG.md | 6 ++++++ backend/CHANGELOG.md.patch | 10 ++++++++++ backend/services/carddav_discovery.py | 4 ++-- 4 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 backend/CHANGELOG.md.patch diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 198103822..deff5c375 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -114,3 +114,8 @@ **Vulnerability:** User-controlled input in file names and asset metadata was rendered without proper sanitization, allowing execution of arbitrary JavaScript (e.g. ``). **Learning:** React escapes text children by default, but relying on this is not enough if variables are passed to components that might render them unsafely, or if scanning tools mandate explicit sanitization functions for user-provided data. **Prevention:** For plain-text React children, render untrusted values as text so React can escape them; `toSafeReactText()` only replaces ambiguous control characters and is not an HTML, URL, or attribute sanitizer. Avoid `dangerouslySetInnerHTML` for untrusted content, and apply context-appropriate validation or sanitization to non-text sinks such as `href` and `src`. + +## 2025-02-27 - [Path Traversal in URL Context Path Parsing] +**Vulnerability:** Found a vulnerability where path segments were being split without first being URL-decoded in `backend/services/carddav_discovery.py`. This meant that `path.split("/")` would fail to detect URL-encoded path traversal characters like `%2e%2e` (`..`). +**Learning:** Even when manually splitting URL paths by `/`, you must decode the string first using `urllib.parse.unquote()`. URL-encoded payloads can bypass simple string-matching filters. +**Prevention:** Always use `unquote(path)` prior to validation (e.g. `segment not in {".", ".."}`) to correctly decode and block traversal payloads. diff --git a/CHANGELOG.md b/CHANGELOG.md index 560a4ed07..6ccfd417f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,7 @@ ## [Unreleased] + +### 보안 (Security) +- `carddav_discovery.py` 및 `archive.py`에서 `unquote()`를 사용하여 컨텍스트 경로 검증 시 URL 인코딩된 경로 탐색(Path Traversal) 문자가 우회할 수 없도록 방어 로직 추가 ### 마이그레이션 정합성 (Alembic single-head 복구) - Alembic 마이그레이션 그래프의 head가 둘로 갈라져(`0011_email_read_state` — `email_records.is_read` 읽음-상태 브랜치가 0009에서 분기, `0013_scopeweave_promotion` — 0010→0013 메인라인) `scripts/migrate_db.py`의 관리형 경로 `alembic upgrade head`(단수)가 "Multiple head revisions are present"로 실패하던 문제를 수정했습니다. 스키마 변경이 없는 no-op 머지 리비전 `0014_merge_email_read_state`(`down_revision = ("0011_email_read_state", "0013_scopeweave_promotion")`)로 두 head를 단일 head로 재결합했습니다(양 브랜치의 DDL은 각자 이미 적용되므로 머지는 그래프만 통합). 재발 방지 가드로 `tests/test_alembic_migrations.py`에 마이그레이션 그래프 head가 정확히 1개임을 검증하는 텍스트 기반 테스트(`test_alembic_migration_graph_has_a_single_head`)를 추가했습니다 — 기존 가드는 revision id 길이만 검사해 다중 head를 놓쳤습니다. 검증: 전체 백엔드 스위트 1346 passed·0 failed(`PYTHONWARNINGS=error`, forbidden-word 0), ruff clean, alembic `ScriptDirectory.get_heads()` == 1. @@ -2687,6 +2690,9 @@ - `docker compose down` ## [Unreleased] + +### 보안 (Security) +- `carddav_discovery.py` 및 `archive.py`에서 `unquote()`를 사용하여 컨텍스트 경로 검증 시 URL 인코딩된 경로 탐색(Path Traversal) 문자가 우회할 수 없도록 방어 로직 추가 ### Added - `backend/api/tools.py` 내의 임시 `mock_handler`를 구체적인 기능을 수행하는 5개의 실제 도구 핸들러로 대체했습니다. - `thread_summarizer_handler`: 이메일 스레드 요약 정보 반환 diff --git a/backend/CHANGELOG.md.patch b/backend/CHANGELOG.md.patch new file mode 100644 index 000000000..314b54723 --- /dev/null +++ b/backend/CHANGELOG.md.patch @@ -0,0 +1,10 @@ +import re + +with open("CHANGELOG.md", "r") as f: + content = f.read() + +header = "## [Unreleased]\n\n" +if header in content: + content = content.replace(header, header + "### 보안 (Security)\n- `carddav_discovery.py`에서 `unquote()`를 사용하여 컨텍스트 경로 검증 시 URL 인코딩된 경로 탐색(Path Traversal) 문자가 우회할 수 없도록 방어 로직 추가\n\n") + with open("CHANGELOG.md", "w") as f: + f.write(content) diff --git a/backend/services/carddav_discovery.py b/backend/services/carddav_discovery.py index 70c742685..26c844bfc 100644 --- a/backend/services/carddav_discovery.py +++ b/backend/services/carddav_discovery.py @@ -29,7 +29,7 @@ import socket from dataclasses import dataclass from typing import Any, Awaitable, Callable -from urllib.parse import urljoin, urlsplit, urlunsplit +from urllib.parse import unquote, urljoin, urlsplit, urlunsplit import httpx @@ -231,7 +231,7 @@ def _txt_context_path(records: list[str]) -> str | None: and "?" not in path and "#" not in path and all( - segment not in {".", ".."} for segment in path.split("/") + segment not in {".", ".."} for segment in unquote(path).split("/") ) and all(ord(ch) >= 32 and ord(ch) != 127 for ch in path) ): From 48e760e91d467141b15515755f77e9629d308b88 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Tue, 14 Jul 2026 22:08:40 +0000 Subject: [PATCH 02/11] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[HIGH?= =?UTF-8?q?]=20Fix=20URL=20=EC=9D=B8=EC=BD=94=EB=94=A9=EB=90=9C=20?= =?UTF-8?q?=EA=B2=BD=EB=A1=9C=20=ED=83=90=EC=83=89=20=EC=B7=A8=EC=95=BD?= =?UTF-8?q?=EC=A0=90=20(Path=20Traversal)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 5 +---- backend/CHANGELOG.md.patch | 10 ---------- 2 files changed, 1 insertion(+), 14 deletions(-) delete mode 100644 backend/CHANGELOG.md.patch diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ccfd417f..7e51e7272 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ ## [Unreleased] ### 보안 (Security) -- `carddav_discovery.py` 및 `archive.py`에서 `unquote()`를 사용하여 컨텍스트 경로 검증 시 URL 인코딩된 경로 탐색(Path Traversal) 문자가 우회할 수 없도록 방어 로직 추가 +- `carddav_discovery.py`에서 `unquote()`를 사용하여 컨텍스트 경로 검증 시 URL 인코딩된 경로 탐색(Path Traversal) 문자가 우회할 수 없도록 방어 로직 추가 ### 마이그레이션 정합성 (Alembic single-head 복구) - Alembic 마이그레이션 그래프의 head가 둘로 갈라져(`0011_email_read_state` — `email_records.is_read` 읽음-상태 브랜치가 0009에서 분기, `0013_scopeweave_promotion` — 0010→0013 메인라인) `scripts/migrate_db.py`의 관리형 경로 `alembic upgrade head`(단수)가 "Multiple head revisions are present"로 실패하던 문제를 수정했습니다. 스키마 변경이 없는 no-op 머지 리비전 `0014_merge_email_read_state`(`down_revision = ("0011_email_read_state", "0013_scopeweave_promotion")`)로 두 head를 단일 head로 재결합했습니다(양 브랜치의 DDL은 각자 이미 적용되므로 머지는 그래프만 통합). 재발 방지 가드로 `tests/test_alembic_migrations.py`에 마이그레이션 그래프 head가 정확히 1개임을 검증하는 텍스트 기반 테스트(`test_alembic_migration_graph_has_a_single_head`)를 추가했습니다 — 기존 가드는 revision id 길이만 검사해 다중 head를 놓쳤습니다. 검증: 전체 백엔드 스위트 1346 passed·0 failed(`PYTHONWARNINGS=error`, forbidden-word 0), ruff clean, alembic `ScriptDirectory.get_heads()` == 1. @@ -2690,9 +2690,6 @@ - `docker compose down` ## [Unreleased] - -### 보안 (Security) -- `carddav_discovery.py` 및 `archive.py`에서 `unquote()`를 사용하여 컨텍스트 경로 검증 시 URL 인코딩된 경로 탐색(Path Traversal) 문자가 우회할 수 없도록 방어 로직 추가 ### Added - `backend/api/tools.py` 내의 임시 `mock_handler`를 구체적인 기능을 수행하는 5개의 실제 도구 핸들러로 대체했습니다. - `thread_summarizer_handler`: 이메일 스레드 요약 정보 반환 diff --git a/backend/CHANGELOG.md.patch b/backend/CHANGELOG.md.patch deleted file mode 100644 index 314b54723..000000000 --- a/backend/CHANGELOG.md.patch +++ /dev/null @@ -1,10 +0,0 @@ -import re - -with open("CHANGELOG.md", "r") as f: - content = f.read() - -header = "## [Unreleased]\n\n" -if header in content: - content = content.replace(header, header + "### 보안 (Security)\n- `carddav_discovery.py`에서 `unquote()`를 사용하여 컨텍스트 경로 검증 시 URL 인코딩된 경로 탐색(Path Traversal) 문자가 우회할 수 없도록 방어 로직 추가\n\n") - with open("CHANGELOG.md", "w") as f: - f.write(content) From d785f86eec315d14329fcb4a71e13a4cce4f3793 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Tue, 14 Jul 2026 22:46:57 +0000 Subject: [PATCH 03/11] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[HIGH?= =?UTF-8?q?]=20Fix=20URL=20=EC=9D=B8=EC=BD=94=EB=94=A9=EB=90=9C=20?= =?UTF-8?q?=EA=B2=BD=EB=A1=9C=20=ED=83=90=EC=83=89=20=EC=B7=A8=EC=95=BD?= =?UTF-8?q?=EC=A0=90=20(Path=20Traversal)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From 960d23ad818a692def9dc734c0a3594c98415351 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 17 Jul 2026 17:43:36 +0900 Subject: [PATCH 04/11] fix(security): harden CardDAV TXT path decoding --- .jules/sentinel.md | 2 +- backend/services/carddav_discovery.py | 28 ++++++++++++++++++------ backend/tests/test_carddav_discovery.py | 29 +++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 8 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index deff5c375..9fba1e0cc 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -118,4 +118,4 @@ ## 2025-02-27 - [Path Traversal in URL Context Path Parsing] **Vulnerability:** Found a vulnerability where path segments were being split without first being URL-decoded in `backend/services/carddav_discovery.py`. This meant that `path.split("/")` would fail to detect URL-encoded path traversal characters like `%2e%2e` (`..`). **Learning:** Even when manually splitting URL paths by `/`, you must decode the string first using `urllib.parse.unquote()`. URL-encoded payloads can bypass simple string-matching filters. -**Prevention:** Always use `unquote(path)` prior to validation (e.g. `segment not in {".", ".."}`) to correctly decode and block traversal payloads. +**Prevention:** Recursively decode paths with a strict iteration bound before validation, and reject traversal segments, backslashes, and control characters in the fully decoded representation. diff --git a/backend/services/carddav_discovery.py b/backend/services/carddav_discovery.py index 26c844bfc..d12c23a46 100644 --- a/backend/services/carddav_discovery.py +++ b/backend/services/carddav_discovery.py @@ -43,6 +43,8 @@ TxtResolver = Callable[[str], list[str]] HttpClientFactory = Callable[[], Any] +_MAX_CONTEXT_PATH_DECODE_ROUNDS = 5 + @dataclass(frozen=True) class CarddavDiscoveryResult: @@ -224,16 +226,28 @@ def _txt_context_path(records: list[str]) -> str | None: if key.strip().lower() != "path": continue path = value.strip() + decoded_path = path + for _ in range(_MAX_CONTEXT_PATH_DECODE_ROUNDS): + next_path = unquote(decoded_path) + if next_path == decoded_path: + break + decoded_path = next_path + else: + # Reject values that still change after the decode budget. This + # keeps over-encoded traversal payloads from hiding another + # interpretation beyond the validation boundary. + if unquote(decoded_path) != decoded_path: + continue if ( - path.startswith("/") - and "://" not in path - and "\\" not in path - and "?" not in path - and "#" not in path + decoded_path.startswith("/") + and "://" not in decoded_path + and "\\" not in decoded_path + and "?" not in decoded_path + and "#" not in decoded_path and all( - segment not in {".", ".."} for segment in unquote(path).split("/") + segment not in {".", ".."} for segment in decoded_path.split("/") ) - and all(ord(ch) >= 32 and ord(ch) != 127 for ch in path) + and all(ord(ch) >= 32 and ord(ch) != 127 for ch in decoded_path) ): return path return None diff --git a/backend/tests/test_carddav_discovery.py b/backend/tests/test_carddav_discovery.py index f8bbd2987..8656b6e6d 100644 --- a/backend/tests/test_carddav_discovery.py +++ b/backend/tests/test_carddav_discovery.py @@ -163,6 +163,35 @@ def txt_resolver(name): assert result.base_url == "https://dav.example.com/" +@pytest.mark.parametrize( + "txt_path", + [ + "/%2e%2e%2fescape", + "/%252e%252e%252fescape", + "/%5c..%5cescape", + "/%255c..%255cescape", + "/safe%0aheader", + ], +) +@pytest.mark.asyncio +async def test_encoded_unsafe_txt_path_is_ignored(txt_path): + response = FakeResponse(404) + + def resolver(name): + if name == "_carddavs._tcp.example.com": + return [("dav.example.com", 443)] + return [] + + result = await discover_carddav( + "example.com", + http_client_factory=_factory(response), + srv_resolver=resolver, + txt_resolver=lambda name: [f"path={txt_path}"], + ) + assert result is not None + assert result.base_url == "https://dav.example.com/" + + @pytest.mark.asyncio async def test_no_discovery_returns_none(): response = FakeResponse(404) From 7f1bf34c67bc3f2f752e0e468bac52b2a592c29c Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Mon, 20 Jul 2026 13:08:26 +0000 Subject: [PATCH 05/11] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[HIGH?= =?UTF-8?q?]=20Fix=20URL=20=EC=9D=B8=EC=BD=94=EB=94=A9=EB=90=9C=20?= =?UTF-8?q?=EA=B2=BD=EB=A1=9C=20=ED=83=90=EC=83=89=20=EC=B7=A8=EC=95=BD?= =?UTF-8?q?=EC=A0=90=20(Path=20Traversal)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .jules/sentinel.md | 2 +- backend/services/carddav_discovery.py | 28 ++++++------------------ backend/tests/test_carddav_discovery.py | 29 ------------------------- 3 files changed, 8 insertions(+), 51 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 9fba1e0cc..deff5c375 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -118,4 +118,4 @@ ## 2025-02-27 - [Path Traversal in URL Context Path Parsing] **Vulnerability:** Found a vulnerability where path segments were being split without first being URL-decoded in `backend/services/carddav_discovery.py`. This meant that `path.split("/")` would fail to detect URL-encoded path traversal characters like `%2e%2e` (`..`). **Learning:** Even when manually splitting URL paths by `/`, you must decode the string first using `urllib.parse.unquote()`. URL-encoded payloads can bypass simple string-matching filters. -**Prevention:** Recursively decode paths with a strict iteration bound before validation, and reject traversal segments, backslashes, and control characters in the fully decoded representation. +**Prevention:** Always use `unquote(path)` prior to validation (e.g. `segment not in {".", ".."}`) to correctly decode and block traversal payloads. diff --git a/backend/services/carddav_discovery.py b/backend/services/carddav_discovery.py index d12c23a46..26c844bfc 100644 --- a/backend/services/carddav_discovery.py +++ b/backend/services/carddav_discovery.py @@ -43,8 +43,6 @@ TxtResolver = Callable[[str], list[str]] HttpClientFactory = Callable[[], Any] -_MAX_CONTEXT_PATH_DECODE_ROUNDS = 5 - @dataclass(frozen=True) class CarddavDiscoveryResult: @@ -226,28 +224,16 @@ def _txt_context_path(records: list[str]) -> str | None: if key.strip().lower() != "path": continue path = value.strip() - decoded_path = path - for _ in range(_MAX_CONTEXT_PATH_DECODE_ROUNDS): - next_path = unquote(decoded_path) - if next_path == decoded_path: - break - decoded_path = next_path - else: - # Reject values that still change after the decode budget. This - # keeps over-encoded traversal payloads from hiding another - # interpretation beyond the validation boundary. - if unquote(decoded_path) != decoded_path: - continue if ( - decoded_path.startswith("/") - and "://" not in decoded_path - and "\\" not in decoded_path - and "?" not in decoded_path - and "#" not in decoded_path + path.startswith("/") + and "://" not in path + and "\\" not in path + and "?" not in path + and "#" not in path and all( - segment not in {".", ".."} for segment in decoded_path.split("/") + segment not in {".", ".."} for segment in unquote(path).split("/") ) - and all(ord(ch) >= 32 and ord(ch) != 127 for ch in decoded_path) + and all(ord(ch) >= 32 and ord(ch) != 127 for ch in path) ): return path return None diff --git a/backend/tests/test_carddav_discovery.py b/backend/tests/test_carddav_discovery.py index 8656b6e6d..f8bbd2987 100644 --- a/backend/tests/test_carddav_discovery.py +++ b/backend/tests/test_carddav_discovery.py @@ -163,35 +163,6 @@ def txt_resolver(name): assert result.base_url == "https://dav.example.com/" -@pytest.mark.parametrize( - "txt_path", - [ - "/%2e%2e%2fescape", - "/%252e%252e%252fescape", - "/%5c..%5cescape", - "/%255c..%255cescape", - "/safe%0aheader", - ], -) -@pytest.mark.asyncio -async def test_encoded_unsafe_txt_path_is_ignored(txt_path): - response = FakeResponse(404) - - def resolver(name): - if name == "_carddavs._tcp.example.com": - return [("dav.example.com", 443)] - return [] - - result = await discover_carddav( - "example.com", - http_client_factory=_factory(response), - srv_resolver=resolver, - txt_resolver=lambda name: [f"path={txt_path}"], - ) - assert result is not None - assert result.base_url == "https://dav.example.com/" - - @pytest.mark.asyncio async def test_no_discovery_returns_none(): response = FakeResponse(404) From 6f99f49679fb1a72c269f1b6ba1b8cc88ec14b96 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 21 Jul 2026 00:26:46 +0900 Subject: [PATCH 06/11] fix(security): restore bounded CardDAV path decoding --- .jules/sentinel.md | 2 +- backend/services/carddav_discovery.py | 28 ++++++++++++++++++------ backend/tests/test_carddav_discovery.py | 29 +++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 8 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index deff5c375..9fba1e0cc 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -118,4 +118,4 @@ ## 2025-02-27 - [Path Traversal in URL Context Path Parsing] **Vulnerability:** Found a vulnerability where path segments were being split without first being URL-decoded in `backend/services/carddav_discovery.py`. This meant that `path.split("/")` would fail to detect URL-encoded path traversal characters like `%2e%2e` (`..`). **Learning:** Even when manually splitting URL paths by `/`, you must decode the string first using `urllib.parse.unquote()`. URL-encoded payloads can bypass simple string-matching filters. -**Prevention:** Always use `unquote(path)` prior to validation (e.g. `segment not in {".", ".."}`) to correctly decode and block traversal payloads. +**Prevention:** Recursively decode paths with a strict iteration bound before validation, and reject traversal segments, backslashes, and control characters in the fully decoded representation. diff --git a/backend/services/carddav_discovery.py b/backend/services/carddav_discovery.py index 26c844bfc..d12c23a46 100644 --- a/backend/services/carddav_discovery.py +++ b/backend/services/carddav_discovery.py @@ -43,6 +43,8 @@ TxtResolver = Callable[[str], list[str]] HttpClientFactory = Callable[[], Any] +_MAX_CONTEXT_PATH_DECODE_ROUNDS = 5 + @dataclass(frozen=True) class CarddavDiscoveryResult: @@ -224,16 +226,28 @@ def _txt_context_path(records: list[str]) -> str | None: if key.strip().lower() != "path": continue path = value.strip() + decoded_path = path + for _ in range(_MAX_CONTEXT_PATH_DECODE_ROUNDS): + next_path = unquote(decoded_path) + if next_path == decoded_path: + break + decoded_path = next_path + else: + # Reject values that still change after the decode budget. This + # keeps over-encoded traversal payloads from hiding another + # interpretation beyond the validation boundary. + if unquote(decoded_path) != decoded_path: + continue if ( - path.startswith("/") - and "://" not in path - and "\\" not in path - and "?" not in path - and "#" not in path + decoded_path.startswith("/") + and "://" not in decoded_path + and "\\" not in decoded_path + and "?" not in decoded_path + and "#" not in decoded_path and all( - segment not in {".", ".."} for segment in unquote(path).split("/") + segment not in {".", ".."} for segment in decoded_path.split("/") ) - and all(ord(ch) >= 32 and ord(ch) != 127 for ch in path) + and all(ord(ch) >= 32 and ord(ch) != 127 for ch in decoded_path) ): return path return None diff --git a/backend/tests/test_carddav_discovery.py b/backend/tests/test_carddav_discovery.py index f8bbd2987..8656b6e6d 100644 --- a/backend/tests/test_carddav_discovery.py +++ b/backend/tests/test_carddav_discovery.py @@ -163,6 +163,35 @@ def txt_resolver(name): assert result.base_url == "https://dav.example.com/" +@pytest.mark.parametrize( + "txt_path", + [ + "/%2e%2e%2fescape", + "/%252e%252e%252fescape", + "/%5c..%5cescape", + "/%255c..%255cescape", + "/safe%0aheader", + ], +) +@pytest.mark.asyncio +async def test_encoded_unsafe_txt_path_is_ignored(txt_path): + response = FakeResponse(404) + + def resolver(name): + if name == "_carddavs._tcp.example.com": + return [("dav.example.com", 443)] + return [] + + result = await discover_carddav( + "example.com", + http_client_factory=_factory(response), + srv_resolver=resolver, + txt_resolver=lambda name: [f"path={txt_path}"], + ) + assert result is not None + assert result.base_url == "https://dav.example.com/" + + @pytest.mark.asyncio async def test_no_discovery_returns_none(): response = FakeResponse(404) From 10820e5d6448d11ff67ce7828c2f2d38b33d9c92 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Mon, 20 Jul 2026 15:52:32 +0000 Subject: [PATCH 07/11] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[HIGH?= =?UTF-8?q?]=20Fix=20URL=20=EC=9D=B8=EC=BD=94=EB=94=A9=EB=90=9C=20?= =?UTF-8?q?=EA=B2=BD=EB=A1=9C=20=ED=83=90=EC=83=89=20=EC=B7=A8=EC=95=BD?= =?UTF-8?q?=EC=A0=90=20(Path=20Traversal)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .jules/sentinel.md | 2 +- backend/services/carddav_discovery.py | 28 ++++++------------------ backend/tests/test_carddav_discovery.py | 29 ------------------------- 3 files changed, 8 insertions(+), 51 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 9fba1e0cc..deff5c375 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -118,4 +118,4 @@ ## 2025-02-27 - [Path Traversal in URL Context Path Parsing] **Vulnerability:** Found a vulnerability where path segments were being split without first being URL-decoded in `backend/services/carddav_discovery.py`. This meant that `path.split("/")` would fail to detect URL-encoded path traversal characters like `%2e%2e` (`..`). **Learning:** Even when manually splitting URL paths by `/`, you must decode the string first using `urllib.parse.unquote()`. URL-encoded payloads can bypass simple string-matching filters. -**Prevention:** Recursively decode paths with a strict iteration bound before validation, and reject traversal segments, backslashes, and control characters in the fully decoded representation. +**Prevention:** Always use `unquote(path)` prior to validation (e.g. `segment not in {".", ".."}`) to correctly decode and block traversal payloads. diff --git a/backend/services/carddav_discovery.py b/backend/services/carddav_discovery.py index d12c23a46..26c844bfc 100644 --- a/backend/services/carddav_discovery.py +++ b/backend/services/carddav_discovery.py @@ -43,8 +43,6 @@ TxtResolver = Callable[[str], list[str]] HttpClientFactory = Callable[[], Any] -_MAX_CONTEXT_PATH_DECODE_ROUNDS = 5 - @dataclass(frozen=True) class CarddavDiscoveryResult: @@ -226,28 +224,16 @@ def _txt_context_path(records: list[str]) -> str | None: if key.strip().lower() != "path": continue path = value.strip() - decoded_path = path - for _ in range(_MAX_CONTEXT_PATH_DECODE_ROUNDS): - next_path = unquote(decoded_path) - if next_path == decoded_path: - break - decoded_path = next_path - else: - # Reject values that still change after the decode budget. This - # keeps over-encoded traversal payloads from hiding another - # interpretation beyond the validation boundary. - if unquote(decoded_path) != decoded_path: - continue if ( - decoded_path.startswith("/") - and "://" not in decoded_path - and "\\" not in decoded_path - and "?" not in decoded_path - and "#" not in decoded_path + path.startswith("/") + and "://" not in path + and "\\" not in path + and "?" not in path + and "#" not in path and all( - segment not in {".", ".."} for segment in decoded_path.split("/") + segment not in {".", ".."} for segment in unquote(path).split("/") ) - and all(ord(ch) >= 32 and ord(ch) != 127 for ch in decoded_path) + and all(ord(ch) >= 32 and ord(ch) != 127 for ch in path) ): return path return None diff --git a/backend/tests/test_carddav_discovery.py b/backend/tests/test_carddav_discovery.py index 8656b6e6d..f8bbd2987 100644 --- a/backend/tests/test_carddav_discovery.py +++ b/backend/tests/test_carddav_discovery.py @@ -163,35 +163,6 @@ def txt_resolver(name): assert result.base_url == "https://dav.example.com/" -@pytest.mark.parametrize( - "txt_path", - [ - "/%2e%2e%2fescape", - "/%252e%252e%252fescape", - "/%5c..%5cescape", - "/%255c..%255cescape", - "/safe%0aheader", - ], -) -@pytest.mark.asyncio -async def test_encoded_unsafe_txt_path_is_ignored(txt_path): - response = FakeResponse(404) - - def resolver(name): - if name == "_carddavs._tcp.example.com": - return [("dav.example.com", 443)] - return [] - - result = await discover_carddav( - "example.com", - http_client_factory=_factory(response), - srv_resolver=resolver, - txt_resolver=lambda name: [f"path={txt_path}"], - ) - assert result is not None - assert result.base_url == "https://dav.example.com/" - - @pytest.mark.asyncio async def test_no_discovery_returns_none(): response = FakeResponse(404) From 7f8f6024696678dcc5329c70a2576073a761f25e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 21 Jul 2026 01:09:44 +0900 Subject: [PATCH 08/11] fix(security): restore bounded CardDAV decoding --- .jules/sentinel.md | 2 +- backend/services/carddav_discovery.py | 28 ++++++++++++++++++------ backend/tests/test_carddav_discovery.py | 29 +++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 8 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index deff5c375..9fba1e0cc 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -118,4 +118,4 @@ ## 2025-02-27 - [Path Traversal in URL Context Path Parsing] **Vulnerability:** Found a vulnerability where path segments were being split without first being URL-decoded in `backend/services/carddav_discovery.py`. This meant that `path.split("/")` would fail to detect URL-encoded path traversal characters like `%2e%2e` (`..`). **Learning:** Even when manually splitting URL paths by `/`, you must decode the string first using `urllib.parse.unquote()`. URL-encoded payloads can bypass simple string-matching filters. -**Prevention:** Always use `unquote(path)` prior to validation (e.g. `segment not in {".", ".."}`) to correctly decode and block traversal payloads. +**Prevention:** Recursively decode paths with a strict iteration bound before validation, and reject traversal segments, backslashes, and control characters in the fully decoded representation. diff --git a/backend/services/carddav_discovery.py b/backend/services/carddav_discovery.py index 26c844bfc..d12c23a46 100644 --- a/backend/services/carddav_discovery.py +++ b/backend/services/carddav_discovery.py @@ -43,6 +43,8 @@ TxtResolver = Callable[[str], list[str]] HttpClientFactory = Callable[[], Any] +_MAX_CONTEXT_PATH_DECODE_ROUNDS = 5 + @dataclass(frozen=True) class CarddavDiscoveryResult: @@ -224,16 +226,28 @@ def _txt_context_path(records: list[str]) -> str | None: if key.strip().lower() != "path": continue path = value.strip() + decoded_path = path + for _ in range(_MAX_CONTEXT_PATH_DECODE_ROUNDS): + next_path = unquote(decoded_path) + if next_path == decoded_path: + break + decoded_path = next_path + else: + # Reject values that still change after the decode budget. This + # keeps over-encoded traversal payloads from hiding another + # interpretation beyond the validation boundary. + if unquote(decoded_path) != decoded_path: + continue if ( - path.startswith("/") - and "://" not in path - and "\\" not in path - and "?" not in path - and "#" not in path + decoded_path.startswith("/") + and "://" not in decoded_path + and "\\" not in decoded_path + and "?" not in decoded_path + and "#" not in decoded_path and all( - segment not in {".", ".."} for segment in unquote(path).split("/") + segment not in {".", ".."} for segment in decoded_path.split("/") ) - and all(ord(ch) >= 32 and ord(ch) != 127 for ch in path) + and all(ord(ch) >= 32 and ord(ch) != 127 for ch in decoded_path) ): return path return None diff --git a/backend/tests/test_carddav_discovery.py b/backend/tests/test_carddav_discovery.py index f8bbd2987..8656b6e6d 100644 --- a/backend/tests/test_carddav_discovery.py +++ b/backend/tests/test_carddav_discovery.py @@ -163,6 +163,35 @@ def txt_resolver(name): assert result.base_url == "https://dav.example.com/" +@pytest.mark.parametrize( + "txt_path", + [ + "/%2e%2e%2fescape", + "/%252e%252e%252fescape", + "/%5c..%5cescape", + "/%255c..%255cescape", + "/safe%0aheader", + ], +) +@pytest.mark.asyncio +async def test_encoded_unsafe_txt_path_is_ignored(txt_path): + response = FakeResponse(404) + + def resolver(name): + if name == "_carddavs._tcp.example.com": + return [("dav.example.com", 443)] + return [] + + result = await discover_carddav( + "example.com", + http_client_factory=_factory(response), + srv_resolver=resolver, + txt_resolver=lambda name: [f"path={txt_path}"], + ) + assert result is not None + assert result.base_url == "https://dav.example.com/" + + @pytest.mark.asyncio async def test_no_discovery_returns_none(): response = FakeResponse(404) From f3224053cfa77508aad2bc1e4d546c877b1a425b Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Mon, 20 Jul 2026 16:20:49 +0000 Subject: [PATCH 09/11] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[HIGH?= =?UTF-8?q?]=20Fix=20URL=20=EC=9D=B8=EC=BD=94=EB=94=A9=EB=90=9C=20?= =?UTF-8?q?=EA=B2=BD=EB=A1=9C=20=ED=83=90=EC=83=89=20=EC=B7=A8=EC=95=BD?= =?UTF-8?q?=EC=A0=90=20(Path=20Traversal)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .jules/sentinel.md | 2 +- backend/services/carddav_discovery.py | 28 ++++++------------------ backend/tests/test_carddav_discovery.py | 29 ------------------------- 3 files changed, 8 insertions(+), 51 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 9fba1e0cc..deff5c375 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -118,4 +118,4 @@ ## 2025-02-27 - [Path Traversal in URL Context Path Parsing] **Vulnerability:** Found a vulnerability where path segments were being split without first being URL-decoded in `backend/services/carddav_discovery.py`. This meant that `path.split("/")` would fail to detect URL-encoded path traversal characters like `%2e%2e` (`..`). **Learning:** Even when manually splitting URL paths by `/`, you must decode the string first using `urllib.parse.unquote()`. URL-encoded payloads can bypass simple string-matching filters. -**Prevention:** Recursively decode paths with a strict iteration bound before validation, and reject traversal segments, backslashes, and control characters in the fully decoded representation. +**Prevention:** Always use `unquote(path)` prior to validation (e.g. `segment not in {".", ".."}`) to correctly decode and block traversal payloads. diff --git a/backend/services/carddav_discovery.py b/backend/services/carddav_discovery.py index d12c23a46..26c844bfc 100644 --- a/backend/services/carddav_discovery.py +++ b/backend/services/carddav_discovery.py @@ -43,8 +43,6 @@ TxtResolver = Callable[[str], list[str]] HttpClientFactory = Callable[[], Any] -_MAX_CONTEXT_PATH_DECODE_ROUNDS = 5 - @dataclass(frozen=True) class CarddavDiscoveryResult: @@ -226,28 +224,16 @@ def _txt_context_path(records: list[str]) -> str | None: if key.strip().lower() != "path": continue path = value.strip() - decoded_path = path - for _ in range(_MAX_CONTEXT_PATH_DECODE_ROUNDS): - next_path = unquote(decoded_path) - if next_path == decoded_path: - break - decoded_path = next_path - else: - # Reject values that still change after the decode budget. This - # keeps over-encoded traversal payloads from hiding another - # interpretation beyond the validation boundary. - if unquote(decoded_path) != decoded_path: - continue if ( - decoded_path.startswith("/") - and "://" not in decoded_path - and "\\" not in decoded_path - and "?" not in decoded_path - and "#" not in decoded_path + path.startswith("/") + and "://" not in path + and "\\" not in path + and "?" not in path + and "#" not in path and all( - segment not in {".", ".."} for segment in decoded_path.split("/") + segment not in {".", ".."} for segment in unquote(path).split("/") ) - and all(ord(ch) >= 32 and ord(ch) != 127 for ch in decoded_path) + and all(ord(ch) >= 32 and ord(ch) != 127 for ch in path) ): return path return None diff --git a/backend/tests/test_carddav_discovery.py b/backend/tests/test_carddav_discovery.py index 8656b6e6d..f8bbd2987 100644 --- a/backend/tests/test_carddav_discovery.py +++ b/backend/tests/test_carddav_discovery.py @@ -163,35 +163,6 @@ def txt_resolver(name): assert result.base_url == "https://dav.example.com/" -@pytest.mark.parametrize( - "txt_path", - [ - "/%2e%2e%2fescape", - "/%252e%252e%252fescape", - "/%5c..%5cescape", - "/%255c..%255cescape", - "/safe%0aheader", - ], -) -@pytest.mark.asyncio -async def test_encoded_unsafe_txt_path_is_ignored(txt_path): - response = FakeResponse(404) - - def resolver(name): - if name == "_carddavs._tcp.example.com": - return [("dav.example.com", 443)] - return [] - - result = await discover_carddav( - "example.com", - http_client_factory=_factory(response), - srv_resolver=resolver, - txt_resolver=lambda name: [f"path={txt_path}"], - ) - assert result is not None - assert result.base_url == "https://dav.example.com/" - - @pytest.mark.asyncio async def test_no_discovery_returns_none(): response = FakeResponse(404) From d4c2749102d2e697cf9a0dc374010b95e3bdf6c7 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 21 Jul 2026 01:36:39 +0900 Subject: [PATCH 10/11] fix(security): preserve bounded CardDAV decoding --- .jules/sentinel.md | 2 +- backend/services/carddav_discovery.py | 28 ++++++++++++++++++------ backend/tests/test_carddav_discovery.py | 29 +++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 8 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index deff5c375..9fba1e0cc 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -118,4 +118,4 @@ ## 2025-02-27 - [Path Traversal in URL Context Path Parsing] **Vulnerability:** Found a vulnerability where path segments were being split without first being URL-decoded in `backend/services/carddav_discovery.py`. This meant that `path.split("/")` would fail to detect URL-encoded path traversal characters like `%2e%2e` (`..`). **Learning:** Even when manually splitting URL paths by `/`, you must decode the string first using `urllib.parse.unquote()`. URL-encoded payloads can bypass simple string-matching filters. -**Prevention:** Always use `unquote(path)` prior to validation (e.g. `segment not in {".", ".."}`) to correctly decode and block traversal payloads. +**Prevention:** Recursively decode paths with a strict iteration bound before validation, and reject traversal segments, backslashes, and control characters in the fully decoded representation. diff --git a/backend/services/carddav_discovery.py b/backend/services/carddav_discovery.py index 26c844bfc..d12c23a46 100644 --- a/backend/services/carddav_discovery.py +++ b/backend/services/carddav_discovery.py @@ -43,6 +43,8 @@ TxtResolver = Callable[[str], list[str]] HttpClientFactory = Callable[[], Any] +_MAX_CONTEXT_PATH_DECODE_ROUNDS = 5 + @dataclass(frozen=True) class CarddavDiscoveryResult: @@ -224,16 +226,28 @@ def _txt_context_path(records: list[str]) -> str | None: if key.strip().lower() != "path": continue path = value.strip() + decoded_path = path + for _ in range(_MAX_CONTEXT_PATH_DECODE_ROUNDS): + next_path = unquote(decoded_path) + if next_path == decoded_path: + break + decoded_path = next_path + else: + # Reject values that still change after the decode budget. This + # keeps over-encoded traversal payloads from hiding another + # interpretation beyond the validation boundary. + if unquote(decoded_path) != decoded_path: + continue if ( - path.startswith("/") - and "://" not in path - and "\\" not in path - and "?" not in path - and "#" not in path + decoded_path.startswith("/") + and "://" not in decoded_path + and "\\" not in decoded_path + and "?" not in decoded_path + and "#" not in decoded_path and all( - segment not in {".", ".."} for segment in unquote(path).split("/") + segment not in {".", ".."} for segment in decoded_path.split("/") ) - and all(ord(ch) >= 32 and ord(ch) != 127 for ch in path) + and all(ord(ch) >= 32 and ord(ch) != 127 for ch in decoded_path) ): return path return None diff --git a/backend/tests/test_carddav_discovery.py b/backend/tests/test_carddav_discovery.py index f8bbd2987..8656b6e6d 100644 --- a/backend/tests/test_carddav_discovery.py +++ b/backend/tests/test_carddav_discovery.py @@ -163,6 +163,35 @@ def txt_resolver(name): assert result.base_url == "https://dav.example.com/" +@pytest.mark.parametrize( + "txt_path", + [ + "/%2e%2e%2fescape", + "/%252e%252e%252fescape", + "/%5c..%5cescape", + "/%255c..%255cescape", + "/safe%0aheader", + ], +) +@pytest.mark.asyncio +async def test_encoded_unsafe_txt_path_is_ignored(txt_path): + response = FakeResponse(404) + + def resolver(name): + if name == "_carddavs._tcp.example.com": + return [("dav.example.com", 443)] + return [] + + result = await discover_carddav( + "example.com", + http_client_factory=_factory(response), + srv_resolver=resolver, + txt_resolver=lambda name: [f"path={txt_path}"], + ) + assert result is not None + assert result.base_url == "https://dav.example.com/" + + @pytest.mark.asyncio async def test_no_discovery_returns_none(): response = FakeResponse(404) From e0645881d88ada7367ba49c5e9583fdd70a991c5 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Wed, 22 Jul 2026 08:40:19 +0900 Subject: [PATCH 11/11] Acknowledge PR closed