← Back to PRs

#19675: fix(security): prevent zero-width Unicode chars from bypassing boundary marker sanitization

by williamzujkowski open 2026-02-18 02:49 View on GitHub →
size: S
## Summary - **Fix**: Strip invisible Unicode Format (Cf) category characters from external content before boundary marker detection in `replaceMarkers()` - **Vulnerability**: Zero-width characters (U+200B, U+200C, U+200D, U+00AD, U+2060, U+FEFF, bidi controls) inserted into boundary marker text bypass the regex check in `replaceMarkers()`, causing the function to exit early without sanitizing fake markers - **Impact**: An attacker could craft a visually identical fake `<<<END_EXTERNAL_UNTRUSTED_CONTENT>>>` marker that survives sanitization, potentially breaking the trust boundary between wrapped external content and trusted system context ## Technical Details The `replaceMarkers()` function in `src/security/external-content.ts` uses a regex check (`/external_untrusted_content/i`) as an early-exit optimization. When zero-width characters like U+200B (zero-width space) are inserted into the marker text, they break the regex match without changing the visual appearance of the string. **Before (vulnerable):** ``` EXTERNAL_​UNTRUSTED_​CONTENT (with hidden U+200B after each underscore) ↓ foldMarkerText() → unchanged (only handles fullwidth ASCII + angle brackets) ↓ regex test → FAILS (U+200B splits the keyword) ↓ early return → fake marker NOT sanitized ``` **After (fixed):** ``` EXTERNAL_​UNTRUSTED_​CONTENT (with hidden U+200B) ↓ stripInvisibleChars() → EXTERNAL_UNTRUSTED_CONTENT (zero-width chars removed) ↓ foldMarkerText() → unchanged ↓ regex test → MATCHES ↓ marker replaced with [[MARKER_SANITIZED]] ``` ## Changes - Added `stripInvisibleChars()` function using `/\p{Cf}/gu` regex (Unicode Format category) - Applied stripping in `replaceMarkers()` before fold + regex check - Added 11 new test cases covering: ZWSP, ZWNJ, ZWJ, soft hyphen, word joiner, BOM, bidi controls, combinations, and a realistic attack scenario ## References - [Unicode TR36: Security Considerations](https://unicode.org/reports/tr36/) - [Unicode UTS39: Security Mechanisms](https://unicode.org/reports/tr39/) - [OWASP Input Validation Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/Input_Validation_Cheat_Sheet.html) - CVE-2021-42574 (Trojan Source — related class of Unicode bypass) ## AI Disclosure This fix was developed with assistance from Claude (Anthropic). The author understands the code, has verified correctness through comprehensive testing (40/40 tests passing), and takes responsibility for the implementation. ## Test plan - [x] All 40 existing + new tests pass (`vitest run src/security/external-content.test.ts`) - [x] `pnpm build` succeeds - [x] `pnpm check` formatting passes (pre-existing TS error in unrelated file) - [ ] Verify no regressions in email processing with legitimate multilingual content <!-- greptile_comment --> <h3>Greptile Summary</h3> Security fix that prevents zero-width Unicode Format (Cf) category characters from being used to bypass boundary marker sanitization in `replaceMarkers()`. The fix adds a `stripInvisibleChars()` step (using `\p{Cf}` regex) before marker detection, ensuring that invisible characters like ZWSP, ZWNJ, ZWJ, soft hyphens, BOM, and bidi controls can no longer split marker keywords to evade the regex check. - **Correctness**: The index alignment between `stripped` and `folded` is correct — `foldMarkerText` performs strictly 1:1 character replacement, so regex match positions on `folded` map directly to `stripped`. All slice operations on `stripped` using indices from `folded` are safe. - **Scope**: The stripping applies to all content passed through `wrapExternalContent`, `wrapWebContent`, and `buildSafeExternalPrompt` — including email bodies, web fetch results, browser snapshots, and channel metadata. - **Side effect**: Stripping `\p{Cf}` from the full content body (not just marker regions) will decompose emoji ZWJ sequences and remove bidi controls from RTL text. Since output goes to an LLM rather than a visual renderer, practical impact is minimal, but this is worth documenting. - **Tests**: 11 comprehensive test cases cover individual invisible characters, combinations, and a realistic attack scenario. <h3>Confidence Score: 4/5</h3> - This PR is safe to merge — it's a well-scoped security hardening with correct index alignment and comprehensive test coverage. - Score of 4 reflects: (1) the fix is logically correct — index alignment between `stripped` and `folded` is sound because `foldMarkerText` is a 1:1 char replacement; (2) comprehensive test coverage with 11 new cases; (3) minor concern about stripping ZWJ/bidi from all content (not just markers) which could decompose emoji sequences, though the practical impact is minimal since output goes to an LLM. Deducted 1 point for the undocumented side effect on emoji ZWJ sequences and the unchecked multilingual regression test item. - `src/security/external-content.ts` — verify that stripping `\p{Cf}` from all content (not just marker regions) is acceptable for downstream consumers of `wrapExternalContent` / `wrapWebContent`. <sub>Last reviewed commit: b7d9a12</sub> <!-- greptile_other_comments_section --> <sub>(2/5) Greptile learns from your feedback when you react with thumbs up/down!</sub> <!-- /greptile_comment -->

Most Similar PRs