Repository navigation
Conversation
The RPC server trusts client-supplied tensor descriptors more than it should. Three attack paths exist on a server reachable by an untrusted client, all via GRAPH_COMPUTE: 1. Arbitrary read/write. A view node with buffer=NULL, data=NULL and a valid view_src passes the relaxed create_node check, and ggml_backend_view_init then computes data = view_src->data + view_offs with view_offs never validated anywhere. 2. Remotely triggered server abort. ne[] whose product overflows int64 hits GGML_ASSERT in ggml_new_tensor_impl, and nb[] can wrap ggml_nbytes' size_t arithmetic; both are remotely triggerable aborts (DoS). 3. NULL data dereference. A bufferless non-view node with a compute op passes deserialization and reaches the backend with data=NULL. Fixes: - create_node: when buffer==NULL, reject data!=0 (kept from ggml-org#20908) and also reject view_src==0 with op != GGML_OP_NONE. GGML_OP_NONE stays legal: serialize_tensor zeroes buffer/data for tensors whose buffer lives on a different server, and the cross-server graph in test-rpc-multi-server depends on it. Its data is never touched. - create_node: validate views. view_src must have a buffer, and [view_src->data + view_offs, +nbytes(view)) must lie inside that buffer. - deserialize_tensor: check ne[] with the same cumulative-product rule ggml_new_tensor_impl asserts on, and sanity-check nb[], before ggml_new_tensor_4d so no abort path remains reachable. Verified against an untrusted-client PoC harness (raw sockets): OOB view_offs (0xdeadbeef000) rejected, server stays up ne product overflow (2^32-1)^2 rejected, no abort bufferless compute-op node rejected bufferless node + forged data ptr rejected valid buffered graph accepted in-bounds view (view_offs=64) accepted op=NONE cross-server passthrough accepted And against real client workloads: test-rpc-multi-server.sh: PASS (cross-server graphs, strided nb) llama-cli over RPC (CPU and Metal devices, TinyLlama Q8_0, temp 0, fixed seed): token-identical output vs unpatched server Co-Authored-By: Claude Code <noreply@anthropic.com>
github-actions
Bot
added
the
ggml
|
Hi @las7, thanks for your contribution! Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:
Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below. |
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
1 participant
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
Harden the RPC server's `GRAPH_COMPUTE` path against crafted `rpc_tensor` descriptors sent by an untrusted client. Three issues, all reachable pre-auth by anyone who can connect:
Changes (ggml-rpc.cpp only)
Testing
PoC harness (raw sockets, untrusted client) against the patched server:
Usual-case regression:
🤖 Generated with Claude Code