[kyo-zzz]

Summary

compute_size() (include/ctranslate2/storage_view.h) and the inline product in StorageView::reshape() multiply dimensions without overflow checks, and grow() adds unchecked. The model loader validates shapes with a per-multiplication guard since #2091, but the StorageView API paths still accept shapes whose true product overflows dim_t.

Concretely, a shape of {2^62 + 1, 8} has a true product of 2^65 + 8, which wraps to 8 in int64 arithmetic — verified against current main. reserve(8) then succeeds and the resulting view claims 2^62 + 1 rows while _size is 8, a shape/size inconsistent state that downstream ops cannot reason about. The existing #2094 guards only cover reserve() itself, so they cannot catch a size that wrapped before entering it.

What changed

• compute_size(): reject negative dimensions and apply the same per-multiplication guard the model loader uses (size > max / dim).
• StorageView::reshape(): same guard for its inline known_size *= dim product.
• StorageView::grow(): reject size < 0 and _shape[dim] + size > max before the addition.
• Zero dimensions are checked before any multiplication, so a shape that contains one has size 0 whatever its position ({2^62 + 1, 8, 0} and {0, 2^62 + 1, 8} both give 0) and the remaining dimensions are never multiplied. Negative dimensions are still rejected across the whole shape in that case.
• In reshape(), an unknown dimension (-1) next to a zero dimension is now rejected: the known size is 0 there, and that 0 is the divisor of the compatibility check below. This terminates the process on current main, independently of the overflow guards (StorageView(Shape{8}, DataType::INT32).reshape(Shape{0, -1})).
• Five unit tests in tests/storage_view_test.cc mirroring the Check for overflow in StorageView::reserve before allocation #2094 test style: overflowing shapes and reshapes must throw, and the three zero-dimension rules above are pinned.

This is defense-in-depth consistent with the current hardening wave (#2068, #2073, #2091, #2094). I audited the remaining model-parsing surface while preparing this: consume< is used only in src/models/model.cc, the variable loop is covered by the #2091 guard, string/rank reads are length-bounded, vocabulary.cc reserves are driven by in-memory vector sizes, and the 32-bit consume<uint32_t> * item_size product on the pre-v4 path can only shrink num_bytes, which the downstream int64 equality check against the true product rejects — so I found no other exploitable site.

Testing

• Built with MSVC 19.43, BUILD_TESTS=ON, OPENMP_RUNTIME=COMP, BUILD_SHARED_LIBS=OFF, ENABLE_CPU_DISPATCH=OFF, WITH_CUDA=OFF, WITH_MKL=OFF, WITH_DNNL=OFF.
• The new tests compile into ctranslate2_test. Note: the gtest binary still crashes at startup (0xC0000409, even for --gtest_list_tests) in my local environment, both with ENABLE_CPU_DISPATCH=ON and OFF, and the same crash reproduces on unmodified main — a local toolchain quirk rather than something this patch introduces, so the CI suite is the authoritative check for these tests.
• Because of that, the behavior itself is verified locally by linking a standalone program against the built ctranslate2 library: 15 assertions covering the overflow rejections, the position-independence of zero dimensions in both compute_size() and reshape(), the negative-dimension checks, the -1 next to a zero dimension, and the unchanged happy paths ({16} -> {2, -1} -> {-1, 4} -> {4, 4}, {2, 0, 2}).
• Existing storage_view_test.cc tests are untouched and the diff adds no new failure modes on the happy path (valid shapes are unaffected: the guard only rejects values that would previously wrap).

AI usage disclosure

Per the contribution policy: this fix was developed with AI assistance (analysis of the overflow paths, drafting the guards and tests, and building/verifying locally). I have reviewed every line of the diff, understand the wrap arithmetic and the interaction with the #2091/#2094 guards, and take full responsibility for correctness, performance, and design.

[kyo-zzz]

The 4 CI failures came from the overflow guards added in this PR: they evaluated max / dim before checking the dimension, so a zero dimension divided by zero. On x86_64 that raises SIGFPE (address_sanitizer, dnnl, and python-wheel-test jobs died with exit 136); on AArch64 integer division by zero returns 0, so the comparison saw an overflow and rejected every valid zero-dim shape (ZeroDim, ConcatEmpty, MedianFilterShortAxis).

3864162 skips the division when the dimension is zero (an empty tensor cannot overflow) and adds a regression test covering the case. The failed jobs should turn green on this push.

[jordimas]

The zero-dimension fix still seems to depend on where the zero appears. compute_size({INT64_MAX, 2, 0}) throws before reaching the zero, while compute_size({0, INT64_MAX, 2}) returns 0. The same issue affects reshape().

Could we check for zero dimensions before multiplying, while still validating negative dimensions? A regression test with an overflowing prefix followed by zero would cover this; the current {2^62 + 1, 0} case doesn't overflow before reaching the zero.

[kyo-zzz]

Good catch: skipping the division still leaves the prefix, so the result depended on where the zero sat.

Both sites now scan the shape first: negative dimensions are rejected, a zero dimension is recorded, and the multiplication (with the overflow guard) only runs on shapes that have no zero dimension. compute_size returns 0 for {2^62+1, 8, 0} and for {0, 2^62+1, 8} alike, while {2^62+1, 8} still throws.

Restructuring reshape exposed a second consequence of the same ordering: a new shape that combines a zero dimension with -1 leaves the known size at 0, and that 0 is the divisor of the _size % known_size check below. This crashes on master today, independently of this PR -- StorageView(Shape{8}, DataType::INT32).reshape(Shape{0, -1}) terminates with EXCEPTION_INT_DIVIDE_BY_ZERO (0xC0000094 here, SIGFPE on Linux). reshape now rejects an unknown dimension when the shape also has a zero one, since it can't be recovered from an empty size anyway.

Tests: ZeroDimIsValidWhateverItsPosition covers the overflowing prefix you described (both orders, through the constructor and reshape), ZeroDimDoesNotSkipNegativeCheck pins the negative validation, UnknownDimWithZeroDimThrows covers the division. Verified locally against a Release build of the library; the gtest binary doesn't start on this machine, so the suite itself runs on CI.

[jordimas]

Could you also add tests for the new grow() guard? It would be good to cover a negative increment, addition overflow, and normal growth. A shape like {INT64_MAX, 0} should let you test overflow without allocating a huge buffer.

[kyo-zzz]

Thanks, and for merging master in.

Added in 6feb9b1:

GrowRejectsNegativeIncrement - grow(0, -1) and grow(0, INT64_MIN) on a {4} view. Without the guard these were two different silent bugs: -1 reached resize() and shrank the dimension to 3, and INT64_MIN wrapped the addition. The test also asserts the message, since a negative increment is not an overflow.

GrowRejectsAdditionOverflow - uses the {INT64_MAX, 0} shape you suggested: the zero dimension makes the size 0, so the overflowing first dimension costs no memory. Both grow(0, 1) and grow(0, INT64_MAX) are rejected. On master that first call wraps the addition and leaves the shape as {-9223372036854775808, 0} with no error at all, which is what the guard is for.

GrowIncreasesShape - normal growth on {2, 3}, plus growing a zero dimension back to a positive size.

GrowingToAnUnallocatableSizeIsRejected - one more case that fell out of writing these: INT64_MAX is a valid dimension and INT64_MAX elements still fit in a dim_t, so neither grow() nor compute_size() rejects it, and reserve() is what catches the byte size. The three guards leave no path through to the allocator.

One small product change: grow() had a single message for both rejections, and "leads to an overflowing tensor size" is simply wrong for a negative increment (4 - 1 = 3 doesn't overflow), so it is now two guards with two messages.

Verification: the gtest binary still doesn't start on this machine (exits before printing anything), so I ran the same cases as a standalone program linked against the built library. 8 of those assertions fail against origin/master's storage_view.cc and all pass against this branch. After your merge I re-checked that both src/storage_view.cc and tests/storage_view_test.cc compile against the new baseline; the merge touched neither file, so the runtime results above still apply. The full suite on CI is the authority.

[jordimas]

Thanks!