Skip to content

πŸ›‘οΈ Sentinel: [CRITICAL/HIGH] Fix 이메일 CRLF μΈμ μ…˜ 및 이쀑 ν™•μž₯자 우회 취약점 #1164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
seonghobae wants to merge 2 commits into from
Closed

πŸ›‘οΈ Sentinel: [CRITICAL/HIGH] Fix 이메일 CRLF μΈμ μ…˜ 및 이쀑 ν™•μž₯자 우회 취약점 #1164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c8144a8
πŸ›‘οΈ Sentinel: [CRITICAL/HIGH] Fix 이메일 CRLF μΈμ μ…˜ 및 이쀑 ν™•μž₯자 우회 취약점
seonghobae Jul 27, 2026
0615a8f
πŸ›‘οΈ Sentinel: Fix Trivy CI failures due to vulnerable frontend packages
seonghobae Jul 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .jules/sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,7 @@
**Vulnerability:** User-controlled input in file names and asset metadata was rendered without proper sanitization, allowing execution of arbitrary JavaScript (e.g. `<img src=x onerror=alert(1)>`).
**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`.
## 2024-06-25 - [Fix Email SMTP CRLF Injection & Double Extension Upload]
**Vulnerability:** Attackers could inject arbitrary SMTP commands (e.g. MAIL FROM) using CRLF (\r\n) sequences in email subjects or recipients because `^[^\r\n]*$` validation in Pydantic wasn't catching all edge cases correctly. Attackers could also bypass file upload validations by providing double extensions (e.g., `malicious.exe.eml`).
**Learning:** Pydantic regex patterns might fall short for strict network protocol inputs like SMTP headers if improperly formulated or bypassed. Simple `.endswith()` checks for file uploads fail to prevent embedded dangerous extensions.
**Prevention:** Always use `@field_validator` with explicit `mode="before"` string matching for `chr(10)` and `chr(13)` across all user-controlled email header fields (to, subject, in_reply_to, references). Always tokenize uploaded filenames via `.split(".")` and reject if any segment matches a known dangerous extension (e.g., `.exe`, `.sh`).
Comment on lines +117 to +120
Comment on lines +117 to +120

@coderabbitai coderabbitai Bot Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ“ Maintainability & Code Quality | 🟑 Minor | ⚑ Quick win

Remove the duplicate sentinel entry.

This section duplicates lines 94-97 verbatim, producing duplicate headings and redundant guidance. Keep one authoritative entry.

🧰 Tools
πŸͺ› markdownlint-cli2 (0.23.0)

[warning] 117-117: Multiple headings with the same content

(MD024, no-duplicate-heading)

πŸ€– Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.jules/sentinel.md around lines 117 - 120, Remove the duplicate 2024-06-25
sentinel entry from .jules/sentinel.md, preserving a single authoritative
heading and its guidance. Keep the remaining entry unchanged.

Source: Linters/SAST tools

28 changes: 22 additions & 6 deletions backend/api/emails.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from sqlalchemy import func, or_, select
from db.session import get_db
from db.models import Email
from pydantic import BaseModel, EmailStr, Field
from pydantic import BaseModel, EmailStr, Field, field_validator
import datetime
import time
from typing import Literal
Expand Down Expand Up @@ -577,11 +577,16 @@ async def import_email_files(
uploads: list[EmailImportUpload] = []
for upload in files:
normalized_filename = upload.filename.lower().strip() if upload.filename else ""
if not upload.filename or not (
normalized_filename.endswith(".eml")
or normalized_filename.endswith(".zip")
or normalized_filename.endswith(".mbox")
):
if not upload.filename:
raise HTTPException(status_code=400, detail="invalid_file_type")

allowed_extensions = {".eml", ".zip", ".mbox"}
segments = normalized_filename.split(".")
if len(segments) < 2 or ("." + segments[-1]) not in allowed_extensions:
raise HTTPException(status_code=400, detail="invalid_file_type")
Comment on lines +583 to +586

dangerous_extensions = {".exe", ".sh", ".bat", ".cmd", ".msi", ".vbs", ".scr", ".pif", ".dll", ".com"}
if any(("." + seg) in dangerous_extensions for seg in segments[:-1]):
Comment on lines +584 to +589

@coderabbitai coderabbitai Bot Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ”’ Security & Privacy | 🟠 Major | ⚑ Quick win

Normalize each filename segment before checking dangerous extensions.

strip() only trims the whole filename, so malware.exe .eml produces an earlier segment of exe and bypasses the .exe check. Normalize or reject whitespace-padded segments and add a regression test.

Proposed fix 
-        segments = normalized_filename.split(".")
+        segments = [segment.strip() for segment in normalized_filename.split(".")]
πŸ“ Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
segments = normalized_filename.split(".")
if len(segments) < 2 or ("." + segments[-1]) not in allowed_extensions:
raise HTTPException(status_code=400, detail="invalid_file_type")
dangerous_extensions = {".exe", ".sh", ".bat", ".cmd", ".msi", ".vbs", ".scr", ".pif", ".dll", ".com"}
if any(("." + seg) in dangerous_extensions for seg in segments[:-1]):
segments = [segment.strip() for segment in normalized_filename.split(".")]
if len(segments) < 2 or ("." + segments[-1]) not in allowed_extensions:
raise HTTPException(status_code=400, detail="invalid_file_type")
dangerous_extensions = {".exe", ".sh", ".bat", ".cmd", ".msi", ".vbs", ".scr", ".pif", ".dll", ".com"}
if any(("." + seg) in dangerous_extensions for seg in segments[:-1]):
πŸ€– Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/api/emails.py` around lines 584 - 589, Normalize each segment in the
filename validation flow before comparing against dangerous_extensions,
including trimming whitespace or rejecting whitespace-padded segments so values
like β€œexe ” cannot bypass the check. Update the logic around normalized_filename
and the dangerous_extensions check, and add a regression test covering a
filename such as malware.exe .eml.

raise HTTPException(status_code=400, detail="invalid_file_type")

content = await upload.read(MAX_IMPORT_UPLOAD_BYTES + 1)
Expand Down Expand Up @@ -696,6 +701,17 @@ class SendEmailRequest(BaseModel):
in_reply_to: str | None = None # O3: email threading support
references: str | None = None

@field_validator("to", "subject", "in_reply_to", "references", mode="before")
@classmethod
def reject_crlf(cls, v: str | None) -> str | None:
if v is None:
return v
if not isinstance(v, str):
return v
if chr(10) in v or chr(13) in v:
raise ValueError("Email header fields must not contain newlines")
return v
Comment on lines +704 to +713


@router.post("/send")
async def send_email_endpoint(
Expand Down
5 changes: 3 additions & 2 deletions frontend/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@
"class-variance-authority": "^0.7.1",
"clsx": "^2.1.1",
"lucide-react": "^1.24.0",
"next": "16.2.10",
"next": "16.2.12",
"react": "19.2.7",
"react-dom": "19.2.7",
"react-resizable-panels": "^4.12.0",
"sharp": "0.35.3",
"tailwind-merge": "^3.5.0",
"tailwindcss": "^4",
"tw-animate-css": "^1.4.0",
Expand All @@ -42,7 +43,7 @@
"eslint-config-next": "16.2.10",
"fast-check": "^4.9.0",
"jsdom": "^29.1.0",
"postcss": "^8.5.16",
"postcss": "^8.5.23",

@coderabbitai coderabbitai Bot Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ”’ Security & Privacy | 🟠 Major | ⚑ Quick win

🧩 Analysis chain

🏁 Script executed:

cd frontend
rg -n '"postcss"|postcss:' package.json .pnpmfile.cjs
rg -n 'postcss@|version: 8\.5\.' pnpm-lock.yaml
pnpm list postcss --depth Infinity

Repository: ContextualWisdomLab/naruon

Length of output: 1239

🏁 Script executed:

cd frontend
printf '\n== package.json ==\n'
cat -n package.json | sed -n '40,65p'
printf '\n== .pnpmfile.cjs ==\n'
cat -n .pnpmfile.cjs | sed -n '1,120p'
printf '\n== pnpm-lock postcss hits ==\n'
rg -n 'postcss@8\.5\.|version: 8\.5\.|postcss:' pnpm-lock.yaml

Repository: ContextualWisdomLab/naruon

Length of output: 1545

Align all PostCSS constraints with 8.5.23

frontend/package.json already updates the direct dependency, but overrides, resolutions, and frontend/.pnpmfile.cjs still force ^8.5.16/^8.5.15. Those lower ranges can leave transitive installs below the patched release, so bump all three to 8.5.23 and refresh the lockfile.

πŸ€– Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/package.json` at line 46, Update the PostCSS version constraints in
package.json overrides, package.json resolutions, and the frontend/.pnpmfile.cjs
configuration from the older ranges to 8.5.23, then regenerate the lockfile so
all direct and transitive resolutions align with the patched release.

"typescript": "^6",
"vitest": "^4.1.10"
},
Expand Down
Loading
Loading