-
Notifications
You must be signed in to change notification settings - Fork 0
fix(security): bound /parse request bytes before multipart parsing #812
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.
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
Draft
seonghobae
wants to merge
16
commits into
develop
Choose a base branch
from
sentinel-form-dos-fix-3567587568518170111
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
5a733f2
🛡️ Sentinel: [MEDIUM] Fix 폼 필드 메모리 고갈 (DoS) 취약점 수정
seonghobae 87ecab7
CI 재트리거를 위한 빈 커밋
seonghobae 74e5125
repair(security): restore canonical Sentinel doctrine
seonghobae e347c0f
feat(security): bound raw parse request bytes before multipart parsing
seonghobae cb10a7b
test(security): verify raw body admission before parser allocation
seonghobae 0f5ae81
fix(security): keep body-limit middleware lint-clean
seonghobae 8de4ffb
fix(security): enforce parse body limit before multipart allocation
seonghobae 50aa435
test(security): prove auth-before-body-limit integration and edge paths
seonghobae ed0a696
repair(security): restore exact Sentinel owner doctrine
seonghobae 6382d7d
CI 테스트 실패 수정을 위한 누락된 Docstring 추가
seonghobae b260e0b
CI 재트리거를 위한 빈 커밋
seonghobae 241fd4f
CI 재트리거를 위한 빈 커밋
seonghobae 49e57ba
CI 재트리거를 위한 빈 커밋
seonghobae 4494f2c
CI trivy-fs 실패 수정을 위한 pypdf 의존성 업데이트
seonghobae 6c5ed8d
CI 재트리거를 위한 빈 커밋
seonghobae e49ef22
CI trivy-fs 실패 수정을 위한 pypdf 의존성 업데이트
seonghobae File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| """ASGI request-body admission limits for parser uploads.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from collections.abc import Awaitable, Callable | ||
|
|
||
| from starlette.responses import JSONResponse | ||
| from starlette.types import Message, Receive, Scope, Send | ||
|
|
||
| ASGIApp = Callable[[Scope, Receive, Send], Awaitable[None]] | ||
| PAYLOAD_TOO_LARGE_DETAIL = "Payload Too Large" | ||
|
|
||
|
|
||
| class RequestBodyTooLarge(Exception): | ||
| """Signal that actual ASGI request bytes crossed the configured admission cap.""" | ||
|
|
||
|
|
||
| def _declared_content_length(scope: Scope) -> int | None: | ||
| """Return one valid non-negative Content-Length value, otherwise no hint.""" | ||
|
|
||
| values = [ | ||
| value | ||
| for name, value in scope.get("headers", []) | ||
| if name.lower() == b"content-length" | ||
| ] | ||
| if len(values) != 1: | ||
| return None | ||
| try: | ||
| declared = int(values[0]) | ||
| except (TypeError, ValueError): | ||
| return None | ||
| return declared if declared >= 0 else None | ||
|
|
||
|
|
||
| class RequestBodyLimitMiddleware: | ||
| """Bound raw request bytes for one HTTP method/path before body parsing. | ||
|
|
||
| `Content-Length` is only an early-rejection hint. Enforcement always wraps the | ||
| ASGI receive channel and counts actual bytes, so omitted or understated headers | ||
| cannot bypass the limit. The middleware is intended to sit inside the existing | ||
| authentication boundary and outside FastAPI's multipart parsing for `/parse`. | ||
|
|
||
| Remove this compatibility middleware after the repository adopts Starlette | ||
| 1.6+ and its native `max_body_size` / `RequestBodyLimitMiddleware` contract. | ||
| """ | ||
|
|
||
| def __init__( | ||
| self, | ||
| app: ASGIApp, | ||
| *, | ||
| max_body_size: int, | ||
| path: str, | ||
| method: str = "POST", | ||
| ) -> None: | ||
| """Bind an ASGI app to a non-negative byte limit and exact route selector.""" | ||
|
|
||
| if max_body_size < 0: | ||
| raise ValueError("max_body_size must be non-negative") | ||
| self.app = app | ||
| self.max_body_size = max_body_size | ||
| self.path = path | ||
| self.method = method.upper() | ||
|
|
||
| async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: | ||
| """Reject an oversized selected request before downstream body parsing.""" | ||
|
|
||
| if ( | ||
| scope["type"] != "http" | ||
| or scope.get("method", "").upper() != self.method | ||
| or scope.get("path") != self.path | ||
| ): | ||
| await self.app(scope, receive, send) | ||
| return | ||
|
|
||
| declared_length = _declared_content_length(scope) | ||
| if declared_length is not None and declared_length > self.max_body_size: | ||
| await self._send_too_large(scope, receive, send) | ||
| return | ||
|
|
||
| received_bytes = 0 | ||
| response_started = False | ||
|
|
||
| async def limited_receive() -> Message: | ||
| """Wrap receive to enforce the maximum body size limit.""" | ||
| nonlocal received_bytes | ||
| message = await receive() | ||
| if message["type"] == "http.request": | ||
| received_bytes += len(message.get("body", b"")) | ||
| if received_bytes > self.max_body_size: | ||
| raise RequestBodyTooLarge | ||
| return message | ||
|
|
||
| async def tracking_send(message: Message) -> None: | ||
| """Wrap send to track if a response has already started.""" | ||
| nonlocal response_started | ||
| if message["type"] == "http.response.start": | ||
| response_started = True | ||
| await send(message) | ||
|
|
||
| try: | ||
| await self.app(scope, limited_receive, tracking_send) | ||
| except RequestBodyTooLarge: | ||
| if response_started: | ||
| raise | ||
| await self._send_too_large(scope, receive, send) | ||
|
|
||
| @staticmethod | ||
| async def _send_too_large(scope: Scope, receive: Receive, send: Send) -> None: | ||
| """Emit the service's sanitized 413 response through the ASGI interface.""" | ||
|
|
||
| response = JSONResponse( | ||
| status_code=413, | ||
| content={"detail": PAYLOAD_TOO_LARGE_DETAIL}, | ||
| ) | ||
| await response(scope, receive, send) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
Repository: ContextualWisdomLab/newsdom-api
Length of output: 17805
🏁 Script executed:
Repository: ContextualWisdomLab/newsdom-api
Length of output: 9368
Denial of Service (CWE-400): Uncontrolled Resource Consumption
Reachability: External · Exploitability: Moderate
multipart 파싱 단계에도 크기 제한을 추가하세요.
Form(max_length=50)은 multipart 파싱 이후에 적용됩니다. 인증된 호출자도 큰language또는mode파트를 보내 파싱 중 메모리를 소모할 수 있습니다. multipart 파서 또는 ingress에서 파트 크기와 전체 body 크기를 먼저 제한하세요.Form(max_length=50)은 문자 길이 검증으로 유지하세요.🤖 Prompt for AI Agents