Guard against 64-bit overflow in ComputePitch

[Roland Shum (ShumWengSang)]

Guard against 64-bit overflow in ComputePitch

Summary

ComputePitch performs its row-pitch and slice-pitch computations in 64-bit arithmetic, but only validates the result on 32-bit platforms:

#if defined(_M_IX86) || defined(_M_ARM) || defined(_M_HYBRID_X86_ARM64)
    static_assert(sizeof(size_t) == 4, "Not a 32-bit platform!");
    if (pitch > UINT32_MAX || slice > UINT32_MAX)
    {
        rowPitch = slicePitch = 0;
        return HRESULT_E_ARITHMETIC_OVERFLOW;
    }
#else
    static_assert(sizeof(size_t) == 8, "Not a 64-bit platform!");
#endif

On 32-bit targets that check is load-bearing, because size_t cannot represent the result. On 64-bit targets there is no validation at all — the static_assert is a compile-time assertion, not a runtime guard. Sufficiently large dimensions therefore wrap the pitch * height multiply modulo 2^64 and ComputePitch returns S_OK with a slice pitch that is much smaller than the row pitch.

That breaks an invariant the rest of the library depends on:

slicePitch == rowPitch * ComputeScanlines(fmt, height)

Callers rely on the slice pitch both to size allocations and to bounds-check input, while the copy loops are driven by the row pitch and scanline count. When the two disagree, the size check validates against a value that no longer describes the copy that follows:

• DetermineImageArray (DirectXTexImage.cpp:66) accumulates slicePitch into pixelSize
• ScratchImage::Initialize (DirectXTexImage.cpp:370) allocates pixelSize
• CopyImage (DirectXTexDDS.cpp:1555) gates on if (pixelSize > size)
• the scanline loop (DirectXTexDDS.cpp:1654) then copies rowPitch bytes per row

Reachable from any loader that accepts dimensions larger than the D3D limits — e.g. the DDS loader under DDS_FLAGS_ALLOW_LARGE_FILES, which bypasses the 16384 dimension cap in DecodeDDSHeader (DirectXTexDDS.cpp:649-665). texconv, texassemble and texdiag all set that flag unconditionally for .dds input (texconv.cpp:2078, texassemble.cpp:1415/1442/1465, texdiag.cpp:550).

Change

Every multiply in ComputePitch now goes through a small checked helper that records overflow in a local flag; the flag is tested once, next to the existing 32-bit check, and returns HRESULT_E_ARITHMETIC_OVERFLOW.

inline uint64_t MulOverflow(uint64_t a, uint64_t b, bool& overflow) noexcept
{
#if defined(__GNUC__) || defined(__clang__)
    uint64_t result = 0;
    if (__builtin_mul_overflow(a, b, &result)) { overflow = true; return 0; }
    return result;
#else
    const uint64_t result = a * b;
    if ((a != 0) && ((result / a) != b)) { overflow = true; return 0; }
    return result;
#endif
}

MSVC has no __builtin_mul_overflow, so it takes the division-based path; clang-cl, clang and GCC take the intrinsic. <intsafe.h> was deliberately not used since the library also builds for WSL/Linux and macOS. This is not on a hot path — ComputePitch is called once per subresource, not per pixel.

This detects overflow; it does not impose a magnitude cap. That distinction matters. Simply removing the _M_IX86 gate so the existing > UINT32_MAX check applies everywhere would also stop the wrap, but it would reject slice pitches above 4 GB on 64-bit — which are legal and reachable well inside D3D limits. A 16384 x 16384 R32G32B32A32_FLOAT surface (the D3D12 maximum 2D dimension, requiring no special flags) has a slice pitch of exactly 2^32 — one byte over UINT32_MAX. Capping would break loading it on x64. The check added here leaves it working.

Validation

Built x64 Release via CMake, plus a standalone /W4 compile and a clang-cl -Wall -Wextra compile to exercise both helper branches. No new diagnostics.

To confirm the change is purely additive, ComputePitch was swept over 107,712 combinations — 17 formats covering every branch of the switch (BC, packed, planar, and the default bpp path), 24 dimensions each for width and height (powers of two, off-by-ones, 16384/16385, 0x80000000, 0xFFFFFFFF, and values chosen to wrap), and all 11 CP_FLAGS — with the results diffed against the unpatched build:

	count
total cases	107,712
differing	1,562
...of which S_OK → HRESULT_E_ARITHMETIC_OVERFLOW	1,562 (100%)
...any other change	0
cases still returning S_OK	98,230
...byte-identical to unpatched	98,230 (100%)

Every case that previously succeeded without overflowing returns bit-identical values. The only behavioural change is that inputs which used to silently wrap now fail cleanly. Spot checks:

case	before	after
R32G32B32A32_FLOAT 16384×16384	S_OK 262144 / 4294967296	unchanged
NV12 65536×65536	S_OK 65536 / 6442450944	unchanged
BC7_UNORM 16384×16384	S_OK 65536 / 268435456	unchanged
overflowing dimensions	S_OK 4295098370 / 4	HRESULT_E_ARITHMETIC_OVERFLOW

Notes for reviewers

• CP_FLAGS_LIMIT_4GB does not help here. The check in DetermineImageArray (DirectXTexImage.cpp:127) tests totalPixelSize, which is the sum of already-wrapped slice pitches. It runs downstream of the wrap and cannot observe it.
• The accumulator in DetermineImageArray looks similar but is already defended. SetupImageArray (DirectXTexImage.cpp:197-201) walks pixels += slicePitch and fails as soon as the running pointer passes pEndBits, which catches a wrapped total incrementally. No change made there.
• A slicePitch == rowPitch * ComputeScanlines(...) assertion in CopyImage was considered and rejected. The invariant holds for every format except under CP_FLAGS_BAD_DXTN_TAILS, which computes height >> 2 where ComputeScanlines returns max(1, (height + 3) / 4); those differ for any height that is not a multiple of 4, so the assertion would fire on legitimate legacy DXTn content.
• Non-multiplicative edges left alone. A few additions (+ 3u, + 32767u, height + ((height + 1) >> 1)) could in principle wrap, but only for size_t inputs at or near 2^64, which no image format can express and no loader can produce — dimensions are at most 32-bit on every input path. Flagging in case you'd prefer an explicit input bound instead.
• Regression test belongs in walbourn/directxtextest, since that is where the suite lives; happy to open a companion PR adding the overflow cases to the ComputePitch unit tests.
• Fuzzing would not have found this. fuzzloaders already sets DDS_FLAGS_ALLOW_LARGE_FILES | DDS_FLAGS_PERMISSIVE (fuzzloaders.cpp:59), so the affected configuration has been under coverage-guided fuzzing for years. The wrap opens no new edge — a wrapped slice pitch follows exactly the same path as a legitimately small surface — so there is no coverage signal to hill-climb on, and unsigned wrap is well-defined so ASan cannot see it. The interesting value is not in either dimension field but in their product, which mutation is unlikely to land on by chance.

[Chuck Walbourn (walbourn)]

The function uses 64-bit integer math already to deal with overflow detection. If there are specific input values that overflow, please provide some examples so we can verify any fix/change here.

I think the majority of this PR is unnecessary. The one change I do believe may be needed is an initial bounds-check on the size width/height values coming in for 64-bit builds (for 32-bit builds is already going to be bounded by UINT32_MAX). In practice, most of the calling code has already done the bounds check but it's reasonable to add it here since ComputePitch is a public-facing API.

IOW, the only change I think is needed here is:

_Use_decl_annotations_
HRESULT DirectX::ComputePitch(DXGI_FORMAT fmt, size_t width, size_t height,
    size_t& rowPitch, size_t& slicePitch, CP_FLAGS flags) noexcept
{
    uint64_t pitch = 0;
    uint64_t slice = 0;

    if (width > UINT32_MAX || height > UINT32_MAX)
        return E_INVALIDARG;

    switch (static_cast<int>(fmt))
    {
    case DXGI_FORMAT_UNKNOWN:
        return E_INVALIDARG;

[Chuck Walbourn (walbourn)]

Most of this is not needed. You are free to submit a revision that only adds the initial bounds check to make sure the values aren't exceeding 32-bit to begin with.