Skip to content

Bound tensor data offset/size against the mapping in gguf_get_tensor (follow-up to #28)#33

Open
gigioneggiando wants to merge 1 commit into
antirez:mainfrom
gigioneggiando:fix-tensor-data-offset-bounds
Open

Bound tensor data offset/size against the mapping in gguf_get_tensor (follow-up to #28)#33
gigioneggiando wants to merge 1 commit into
antirez:mainfrom
gigioneggiando:fix-tensor-data-offset-bounds

Conversation

@gigioneggiando

Copy link
Copy Markdown

Follow-up to #28, as suggested there.

#28 bounds the metadata / tensor-info walk, but the tensor data region is still unchecked. In gguf_get_tensor():

tensor->offset = ctx->data_off + *offset;          // *offset is raw, file-controlled
tensor->weights_data = ctx->data + tensor->offset; // pointer formed with no bound vs ctx->size

*offset and the resulting tensor->bsize are never checked against ctx->size, so a crafted tensor offset/size places weights_data (or its bsize-byte extent) outside the mapping. Any later read of the weights (gguf_tensor_to_float, inspect-tensor, compare) is then an out-of-bounds read.

Reproduction

A one-tensor GGUF whose data offset is 1 MiB (well past the file), driven through the real gguf_get_tensor:

[poc] file size=57, crafted tensor data offset=0x100000
before: gguf_get_tensor ACCEPTED it (return 1); weights_data is OUTSIDE the mapping -> OOB read primitive
after : gguf_get_tensor REJECTED the out-of-range offset (return 0)

Fix

Compute bsize first, then verify (overflow-safe) that *offset and the resulting absolute offset are within ctx->size and that the bsize-byte extent fits, before weights_data is formed; return 0 otherwise.

This touches only the offset/bsize tail of gguf_get_tensor, not the dimension/ndim area #28 changes, so it should apply cleanly alongside #28 (happy to rebase on top of it if you merge #28 first).

Follow-up to antirez#28. That PR bounds the metadata / tensor-info walk, but the tensor
DATA region is still unchecked: gguf_get_tensor computes
tensor->offset = ctx->data_off + *offset and tensor->weights_data =
ctx->data + tensor->offset from a raw file-supplied 64-bit offset, with no check
that the range stays inside the mapping. Any later read of the weights
(gguf_tensor_to_float, inspect-tensor, compare) is then an out-of-bounds read.

Compute bsize first, then verify (overflow-safe) that *offset and the resulting
absolute offset are within ctx->size and that the bsize-byte extent fits, before
forming weights_data. Reject with return 0 otherwise.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant