Fix signed shape-product wrap in safetensors reader (DoS via NULL deref) by x14ngch3n · Pull Request #1875 · leejet/stable-diffusion.cpp

x14ngch3n · 2026-08-12T17:37:35Z

Summary

read_safetensors_file (src/model_io/safetensors_io.cpp) parses each tensor dimension with shape[i].get<int64_t>() and stores it directly into ne[] with no range or overflow check. TensorStorage::nelements() (src/model_io/tensor_storage.h) multiplies the dims as a signed int64 product, and the reader uses nbytes() (derived from that product) for its only size check:

tensor_size_ok = (tensor_storage.nbytes() == tensor_data_size);   // safetensors_io.cpp:346
// tensor_data_size = end - begin, already bounded against the file at :255

A crafted .safetensors with shape = [4294967296, 4294967296] makes the int64 product overflow to exactly 0, so nbytes() == 0. Pairing it with an empty, in-bounds data range (data_offsets [x, x]) makes 0 == 0 pass the size check, and the tensor is accepted with huge ne[] but nbytes() == 0.

Impact — ASAN-confirmed NULL deref (DoS)

ggml_new_tensor computes its allocation size from the same ne[] and also wraps to 0, returning a tensor whose data pointer is NULL (ggml_nbytes == 0, data == 0x0). The first consumer access then dereferences NULL → crash.

read_safetensors_file: ACCEPTED (BUG)
tensor 't': nelements()=0 nbytes()=0 ne=[4294967296,4294967296]
ggml_new_tensor OK: ggml_nbytes=0 data=0x0
==95833==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000
==95833==The signal is caused by a READ memory access.
==95833==Hint: address points to the zero page.

Impact scope: denial of service (crash) via a malicious model file. Only an exact-zero wrap is reachable — a near-wrap to a small nonzero product is infeasible with IEEE-double-representable dims (a·b = N·2^64 + k with both a,b < 2^53), so the consequence is a NULL-deref crash, not a heap out-of-bounds. Negative dimensions were also accepted at parse time and reached the int64 product.

The fix

Validate each parsed dimension (reject <= 0) and compute the element-count product with __builtin_mul_overflow, rejecting the tensor up front if any dim is non-positive or the product overflows. This mirrors the invariants the downstream ggml/tensor paths assume hold.

for (int i = 0; i < n_dims; i++) {
    ne[i] = shape[i].get<int64_t>();
    if (ne[i] <= 0) { set_error(...); return false; }
}
// overflow-checked product
int64_t product = 1;
for (int i = 0; i < n_dims; i++) {
    if (__builtin_mul_overflow(product, ne[i], &product)) { set_error(...); return false; }
}
if (product <= 0) { set_error(...); return false; }

Verification

AddressSanitizer (-O1) harnesses against this reader:

shape = [2^32, 2^32] (was: ACCEPTED → NULL deref) now REJECTED — "tensor 't' has an overflowed/invalid shape product".
shape = [-1, 2] (negative dim) now REJECTED — "dimension 0 is -1".
legitimate tensor [2, 2] still accepted (nelements=4) — no regression.

Repro harness and PoC tensors available on request.

read_safetensors_file parsed each tensor dimension with shape[i].get<int64_t>() and stored it directly into ne[] with no range or overflow check. TensorStorage::nelements() (tensor_storage.h) then multiplies the dims as a signed int64 product, and read_safetensors_file uses nbytes() (derived from that product) for its only size check: tensor_size_ok = (tensor_storage.nbytes() == tensor_data_size); with tensor_data_size = end - begin, already bounded against the file. A crafted .safetensors with shape [2^32, 2^32] makes the int64 product overflow to exactly 0, so nbytes() == 0. Pairing it with an empty, in-bounds data range (data_offsets [x, x]) makes 0 == 0 pass the size check, and the tensor is accepted with huge ne[] but nbytes() == 0. ggml_new_tensor computes its allocation size from the same ne[] and also wraps to 0, returning a tensor whose data pointer is NULL. The first consumer access then dereferences NULL -> crash (denial of service). Only an exact-zero wrap is reachable (a near-wrap to a small nonzero product is infeasible with IEEE-double-representable dims), so the impact is a NULL-deref DoS, not a heap out-of-bounds. Negative dimensions were also accepted at parse time and reached the int64 product; they happened to be caught by the size check only when the file range did not coincidentally match. Fix: validate each parsed dimension (reject <= 0) and compute the element-count product with __builtin_mul_overflow, rejecting the tensor up front if any dim is non-positive or the product overflows. This mirrors the checks the upstream ggml/tensor paths assume hold. Verified with AddressSanitizer (-O1) harnesses against this reader: - shape [2^32, 2^32] (was: ACCEPTED -> NULL deref) now REJECTED with "overflowed/invalid shape product". - shape [-1, 2] (negative dim) now REJECTED with a clear message. - legitimate tensor [2, 2] still accepted (nelements=4, no regression). Co-Authored-By: Claude <noreply@anthropic.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7a03cf0efd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-08-12T17:41:03Z

+            int64_t product = 1;
+            bool overflow   = false;
+            for (int i = 0; i < n_dims && !overflow; i++) {
+                if (__builtin_mul_overflow(product, ne[i], &product)) {


Validate byte-size overflow as well as element-count overflow

A crafted F32 tensor with shape [4611686018427387904] still passes this check because its element count fits in int64_t, but TensorStorage::nbytes() subsequently multiplies it by 4 and wraps to zero. With equal in-bounds data offsets, the size check therefore accepts the tensor and preserves the zero-byte/huge-dimension state that leads to the NULL-buffer dereference this change intends to prevent. Validate the required byte count with overflow detection before constructing the storage.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-08-12T17:41:03Z

+            // valid tensor shape; zero is handled by the empty-tensor rules
+            // elsewhere and a negative dim would let the int64 shape product
+            // wrap, masking the real (huge) element count.
+            if (ne[i] <= 0) {


Preserve valid empty safetensors tensors

Safetensors permits empty tensors whose shape contains a zero extent and whose data offsets are equal, so rejecting ne[i] == 0 makes an otherwise valid checkpoint or shard fail before ModelLoader can ignore an unused empty entry. Reject negative dimensions and overflowed products, but handle or safely skip zero-sized tensors separately.

Useful? React with 👍 / 👎.

Keep only the invariant note on the n_dims==5 collapse; drop comments that only describe what the code does, per the repo comment policy.

Review feedback on leejet#1875. The element-count product check alone does not cover TensorStorage::nbytes(), which is nelements()*ggml_type_size(type)/ggml_blck_size(type): a tensor whose element count fits in int64_t (e.g. a 2^62-element F32 tensor) overflows only at the byte-size multiply (x4 -> 0), reproducing the zero-size / NULL-buffer path the reader is meant to reject. Add a second overflow check on nelems*ggml_type_size(type) before the storage is constructed. A zero extent is a valid safetensors empty tensor (equal data offsets), so reject only negative dimensions, drop the product<=0 clause, and skip a genuinely empty tensor (nelems == 0) rather than build a zero-byte storage. Co-Authored-By: Claude <noreply@anthropic.com>

x14ngch3n · 2026-08-13T03:50:24Z

Thanks for the review — both points addressed in the latest push (4ed4053).

P1 (byte-size overflow): Added a second __builtin_mul_overflow check on nelems * ggml_type_size(type) before the TensorStorage is constructed. The element-count check alone does not cover TensorStorage::nbytes() = nelements()*ggml_type_size(type)/ggml_blck_size(type), so a tensor whose element count fits in int64_t (e.g. a 2^62-element F32 tensor) overflowed only at the byte-size multiply (×4 → 0) and re-opened the zero-size path. The new check mirrors nbytes()'s numerator, so if it passes the internal multiply cannot wrap.

P2 (zero dimensions): Now rejects only negative dimensions (ne[i] < 0). A zero extent is a valid safetensors empty tensor (equal data offsets), so the product <= 0 clause is dropped and a genuinely empty tensor (nelems == 0) is skipped — same pattern as the existing U8 skip — rather than building a zero-byte storage that ggml would turn into a NULL buffer.

Builds clean (Release).

x14ngch3n mentioned this pull request Aug 12, 2026

Signed shape-product wrap in safetensors reader (NULL deref / DoS, ASAN-confirmed) #1876

Open

chatgpt-codex-connector Bot reviewed Aug 12, 2026

View reviewed changes

x14ngch3n and others added 2 commits August 13, 2026 09:25

Trim verbose comments in safetensors shape-product fix

480e158

Keep only the invariant note on the n_dims==5 collapse; drop comments that only describe what the code does, per the repo comment policy.

Navigation Menu

Fix signed shape-product wrap in safetensors reader (DoS via NULL deref) - #1875

Fix signed shape-product wrap in safetensors reader (DoS via NULL deref)#1875
x14ngch3n wants to merge 3 commits into
leejet:masterfrom
x14ngch3n:worktree-sd-safetensors-fix

x14ngch3n commented Aug 12, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Aug 12, 2026

Uh oh!

chatgpt-codex-connector Bot Aug 12, 2026

Uh oh!

x14ngch3n commented Aug 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

x14ngch3n commented Aug 12, 2026

Summary

Impact — ASAN-confirmed NULL deref (DoS)

The fix

Verification

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

x14ngch3n commented Aug 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant