Fix one-byte out-of-bounds read in BITPOS with explicit end offset

[hexonal]

Problem

BitPosDriver (libs/server/Resp/Bitmap/BitmapManagerBitPos.cs) clamps an explicit end offset to inputLen (BYTE mode) / bitLen (BIT mode) instead of inputLen - 1 / bitLen - 1 — one index past the last valid position:

endOffset = endOffset >= inputLen ? inputLen : endOffset;                 // BYTE mode
...
endOffset = endByteIndex >= inputLen ? bitLen : endOffset;                // BIT mode

BitPosByteSearch/BitPosBitSearch iterate while (currentOffset <= endOffset) over an unsafe byte*, treating endOffset as an inclusive, valid last index (see e.g. remainder = endOffset - currentStartOffset + 1 in the BYTE-mode search). When no matching bit exists within the real data and the loop actually reaches the clamped endOffset, it dereferences input[inputLen] — one byte past the value's backing buffer. Reproducible with e.g. BITPOS key 0 0 1000000 against a short value with no 0-bit... (i.e. any explicit end offset >= the value's real length, on a value with no match).

Fix

Clamp to inputLen - 1 / bitLen - 1 in both branches, matching:

• BitCountDriver's existing (correct) clamp in the same file family, itself fixed for this exact bug class in BITCOUNT Upper bound calculation fix #1138 for BYTE mode but apparently never mirrored to BITPOS (or to BitCountDriver's own BIT-mode path, which was already correct).
• Real Redis's bitposCommand in src/bitops.c, which clamps end = totlen - 1 (totlen already converted to bits in BIT mode) for both indexing modes.

BitPosByteSearch/BitPosBitSearch are private with a single caller (BitPosDriver), itself called from exactly one production call site, so no additional in-loop bounds check is warranted — see full investigation notes for the alternatives considered and rejected.

Testing

Added two regression tests that call BitmapManager.BitPosDriver directly against an unmanaged buffer with a "canary" byte placed immediately past the declared value length (rather than relying on whatever happens to follow the value on the managed heap, which is not reliable enough to deterministically catch a one-byte overread). Confirmed both tests fail against the pre-fix clamp (reporting a position inside the canary byte instead of -1) and pass with the fix.

Full BITPOS test category (14/14) and dotnet format --verify-no-changes pass clean.

Note: while investigating, I found a separate, unrelated pre-existing bug — BitCountDriver's BitIndexCount helper (BIT-mode partial-byte counting) calls the x86-only Popcnt.X64.PopCount intrinsic with no IsSupported/portable fallback, which crashes the server process on ARM64 hosts. Confirmed pre-existing on unmodified main and unrelated to this change; not fixed here to keep this PR scoped to the BITPOS offset-clamp bug — happy to file that separately if useful.

[Copilot]

Pull request overview

Fixes an off-by-one clamp in BitmapManager.BitPosDriver that could allow BITPOS (with an explicit end offset beyond the value length) to read one byte past the end of the backing buffer in both BYTE and BIT offset modes.

Changes:

• Clamp endOffset to inputLen - 1 (BYTE mode) and bitLen - 1 (BIT mode) to keep the inclusive search bound within the valid buffer.
• Add two regression tests that deterministically detect the prior one-byte overread using an unmanaged “canary” byte placed immediately past the declared value length.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
libs/server/Resp/Bitmap/BitmapManagerBitPos.cs	Fixes inclusive endOffset clamping to prevent one-byte out-of-bounds reads in BYTE and BIT search paths.
test/standalone/Garnet.test.complexstring/GarnetBitmapTests.cs	Adds two regression tests using unmanaged buffers + a canary byte to catch the prior off-by-one overread deterministically.

Comments suppressed due to low confidence (1)

libs/server/Resp/Bitmap/BitmapManagerBitPos.cs:51

• Same issue as the BYTE-mode branch: the comment says "return 0" when the start offset is beyond the end offset, but the code returns -1. Please align the comment with the actual return value.

                if (startByteIndex > endByteIndex) // If start offset beyond endOffset return 0
                    return -1;