Make tool-output validation O(n) instead of O(n^2) (pre-auth CPU DoS)#547
Open
gigioneggiando wants to merge 1 commit into
Open
Make tool-output validation O(n) instead of O(n^2) (pre-auth CPU DoS)#547gigioneggiando wants to merge 1 commit into
gigioneggiando wants to merge 1 commit into
Conversation
responses_validate_tool_outputs() and anthropic_validate_tool_results() resolved each tool output's call_id with responses_find_prior_call_msg(), a backward scan over all preceding messages. Run once per id per tool message over an attacker-supplied, uncapped message array, that is O(n^2): ~10^6 messages is ~10^12 comparisons, pinning a CPU core pre-auth. Only assistant messages declare call ids (chat_msg_has_call_id matched assistant .calls entries), so build a call_id -> nearest-preceding assistant message map incrementally with a rax while scanning forward, and resolve each id with an O(1) lookup. Registering assistant messages as they are passed and looking up on tool messages reproduces the previous nearest-preceding-before-i result exactly. Drop the now-unused responses_find_prior_call_msg() and chat_msg_has_call_id(). Verified: the validator's accept/reject results are unchanged, and wall clock now scales ~linearly with n instead of ~4x per doubling.
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
responses_validate_tool_outputs()and its Anthropic siblinganthropic_validate_tool_results()resolved each tool output'scall_idwithresponses_find_prior_call_msg()— a backward scan over all preceding messages. Run once per id per tool message over an attacker-supplied, uncapped message array, this is O(n²): ~10⁶ messages ≈ 10¹² comparisons, pinning a CPU core pre-auth.Fix
Only assistant messages declare call ids (
chat_msg_has_call_idmatched an assistant's.callsentries). So build acall_id → nearest-preceding assistant messagemap incrementally with arax(already used in this file) while scanning forward, and resolve each id with an O(1) lookup.Registering assistant messages as they are passed and looking up on tool messages reproduces the previous "nearest assistant call before index i" result exactly — including the case where the declaring assistant message appears after the tool message (still not matched, since it isn't in the map yet). The now-unused
responses_find_prior_call_msg()andchat_msg_has_call_id()are removed.Verification
Timed the real
responses_validate_tool_outputs()on synthetic message arrays, and confirmed the accept/reject result (ok) is unchanged at every size:Found during a coordinated security review of ds4; a standalone offline PoC is available on request.