Cap file-declared lengths in agent KV cache reads (memory DoS)#544
Open
gigioneggiando wants to merge 1 commit into
Open
Cap file-declared lengths in agent KV cache reads (memory DoS)#544gigioneggiando wants to merge 1 commit into
gigioneggiando wants to merge 1 commit into
Conversation
agent_kv_read_text() and agent_kv_read_title_trailer() read a uint32 length from a persisted agent session/cache file and immediately xmalloc that many bytes. On 64-bit (size_t)len + 1 does not wrap, so there is no overflow, but a 1-byte file declaring 0xFFFFFFFF drives a ~4 GiB allocation before the following fread() fails -- an attacker-influenced cache file becomes a memory-pressure DoS. Add agent_fp_remaining() and reject a length larger than the bytes left in the file before allocating, at both sites.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
agent_kv_read_text()andagent_kv_read_title_trailer()read auint32length from a persisted agent session / cache file and immediatelyxmalloc((size_t)len + 1).On 64-bit there is no
size_twrap (so no overflow), but the length is uncapped: a 1-byte file declaringtext_bytes = 0xFFFFFFFFdrives a ~4 GiB allocation before the followingfread()fails. An attacker-influenced cache/session file thus becomes a memory-pressure DoS.Fix
Add
agent_fp_remaining()(bytes from the current position to EOF, position-preserving) and reject a length larger than the bytes actually left in the file, before allocating — at both sites.Verification
Drove the real
agent_kv_read_text()with a 1-byte file declaringtext_bytes = 0xFFFFFFFF(ASan allocation cap on, so an over-large request is visible instantly):xmallocrequest atagent_kv_read_text.false(truncated cached text) with no large allocation.Found during a coordinated security review of ds4; a standalone offline PoC is available on request. (Companion to the model-loader length-prefix fix — same class, different subsystem.)