From 7a03cf0efd5d64f6c0cfb3e7b13aa9674c4e55e4 Mon Sep 17 00:00:00 2001 From: Xiang Chen Date: Thu, 13 Aug 2026 01:35:30 +0800 Subject: [PATCH 1/3] Fix signed shape-product wrap in safetensors reader (DoS via NULL deref) read_safetensors_file parsed each tensor dimension with shape[i].get() 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 --- src/model_io/safetensors_io.cpp | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/model_io/safetensors_io.cpp b/src/model_io/safetensors_io.cpp index 69bcaa1ec..b676f7f70 100644 --- a/src/model_io/safetensors_io.cpp +++ b/src/model_io/safetensors_io.cpp @@ -272,10 +272,42 @@ bool read_safetensors_file(const std::string& file_path, int64_t ne[SD_MAX_DIMS] = {1, 1, 1, 1, 1}; for (int i = 0; i < n_dims; i++) { ne[i] = shape[i].get(); + // Reject non-positive dimensions. A negative or zero dim is never a + // 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) { + set_error(error, "invalid tensor shape for '" + name + "' (dimension " + + std::to_string(i) + " is " + std::to_string(ne[i]) + ")"); + return false; + } + } + + // Validate the shape product up front with overflow detection. Without + // this, a shape such as [2^32, 2^32] wraps the int64 product to 0, + // which then matches an empty in-bounds data range in the size check + // below and lets the tensor pass validation with huge ne[] but + // nbytes()==0. ggml later allocates a 0-byte (NULL) buffer for those + // dims, so the first consumer access is a NULL pointer dereference. + { + int64_t product = 1; + bool overflow = false; + for (int i = 0; i < n_dims && !overflow; i++) { + if (__builtin_mul_overflow(product, ne[i], &product)) { + overflow = true; + } + } + if (overflow || product <= 0) { + set_error(error, "tensor '" + name + "' has an overflowed/invalid shape product"); + return false; + } } if (n_dims == 5) { n_dims = 4; + // This collapse multiplies ne[0]*ne[1]; the overflow check above + // already proved the full product is in range, so the partial + // product here cannot overflow. ne[0] = ne[0] * ne[1]; ne[1] = ne[2]; ne[2] = ne[3]; From 480e1588468cdb348a5e5228b059d556c5281d6a Mon Sep 17 00:00:00 2001 From: Xiang Chen Date: Thu, 13 Aug 2026 09:25:01 +0800 Subject: [PATCH 2/3] Trim verbose comments in safetensors shape-product fix 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. --- src/model_io/safetensors_io.cpp | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/model_io/safetensors_io.cpp b/src/model_io/safetensors_io.cpp index b676f7f70..0ff7eaf2b 100644 --- a/src/model_io/safetensors_io.cpp +++ b/src/model_io/safetensors_io.cpp @@ -272,10 +272,6 @@ bool read_safetensors_file(const std::string& file_path, int64_t ne[SD_MAX_DIMS] = {1, 1, 1, 1, 1}; for (int i = 0; i < n_dims; i++) { ne[i] = shape[i].get(); - // Reject non-positive dimensions. A negative or zero dim is never a - // 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) { set_error(error, "invalid tensor shape for '" + name + "' (dimension " + std::to_string(i) + " is " + std::to_string(ne[i]) + ")"); @@ -283,12 +279,6 @@ bool read_safetensors_file(const std::string& file_path, } } - // Validate the shape product up front with overflow detection. Without - // this, a shape such as [2^32, 2^32] wraps the int64 product to 0, - // which then matches an empty in-bounds data range in the size check - // below and lets the tensor pass validation with huge ne[] but - // nbytes()==0. ggml later allocates a 0-byte (NULL) buffer for those - // dims, so the first consumer access is a NULL pointer dereference. { int64_t product = 1; bool overflow = false; @@ -305,9 +295,7 @@ bool read_safetensors_file(const std::string& file_path, if (n_dims == 5) { n_dims = 4; - // This collapse multiplies ne[0]*ne[1]; the overflow check above - // already proved the full product is in range, so the partial - // product here cannot overflow. + // partial product; full product already overflow-checked above ne[0] = ne[0] * ne[1]; ne[1] = ne[2]; ne[2] = ne[3]; From 4ed40534c0cef4af871981f3f11e1fcf08fd5630 Mon Sep 17 00:00:00 2001 From: Xiang Chen Date: Thu, 13 Aug 2026 11:50:00 +0800 Subject: [PATCH 3/3] Check safetensors byte-size overflow; allow empty tensors Review feedback on #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 --- src/model_io/safetensors_io.cpp | 36 +++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/src/model_io/safetensors_io.cpp b/src/model_io/safetensors_io.cpp index 0ff7eaf2b..be20d769e 100644 --- a/src/model_io/safetensors_io.cpp +++ b/src/model_io/safetensors_io.cpp @@ -272,23 +272,37 @@ bool read_safetensors_file(const std::string& file_path, int64_t ne[SD_MAX_DIMS] = {1, 1, 1, 1, 1}; for (int i = 0; i < n_dims; i++) { ne[i] = shape[i].get(); - if (ne[i] <= 0) { + if (ne[i] < 0) { set_error(error, "invalid tensor shape for '" + name + "' (dimension " + - std::to_string(i) + " is " + std::to_string(ne[i]) + ")"); + std::to_string(i) + " is negative)"); return false; } } - { - int64_t product = 1; - bool overflow = false; - for (int i = 0; i < n_dims && !overflow; i++) { - if (__builtin_mul_overflow(product, ne[i], &product)) { - overflow = true; - } + // nelements() is the product of the dims; nbytes() is + // nelements()*ggml_type_size(type)/ggml_blck_size(type). Both multiply + // silently, so a 2^62-element F32 tensor overflows only at the byte size + // (x4 -> 0) and reproduces the NULL-buffer deref this reader rejects. + // Check both with overflow detection; a zero extent is a valid empty + // tensor, so skip it rather than build a zero-byte storage. + int64_t nelems = 1; + bool overflow = false; + for (int i = 0; i < n_dims && !overflow; i++) { + if (__builtin_mul_overflow(nelems, ne[i], &nelems)) { + overflow = true; } - if (overflow || product <= 0) { - set_error(error, "tensor '" + name + "' has an overflowed/invalid shape product"); + } + if (overflow) { + set_error(error, "tensor '" + name + "' has an overflowed shape product"); + return false; + } + if (nelems == 0) { + continue; + } + { + int64_t nbytes_num; + if (__builtin_mul_overflow(nelems, (int64_t)ggml_type_size(type), &nbytes_num)) { + set_error(error, "tensor '" + name + "' byte size overflows"); return false; } }