diff --git a/libs/server/Resp/Bitmap/BitmapManagerBitPos.cs b/libs/server/Resp/Bitmap/BitmapManagerBitPos.cs index ff8f3237598..e425fc14c1e 100644 --- a/libs/server/Resp/Bitmap/BitmapManagerBitPos.cs +++ b/libs/server/Resp/Bitmap/BitmapManagerBitPos.cs @@ -28,10 +28,10 @@ public static long BitPosDriver(byte* input, int inputLen, long startOffset, lon if (startOffset >= inputLen) // If startOffset greater that valLen always bitpos -1 return -1; - if (startOffset > endOffset) // If start offset beyond endOffset return 0 + if (startOffset > endOffset) // If start offset beyond endOffset return -1 return -1; - endOffset = endOffset >= inputLen ? inputLen : endOffset; + endOffset = endOffset >= inputLen ? inputLen - 1 : endOffset; // BYTE search return BitPosByteSearch(input, inputLen, startOffset, endOffset, searchFor); } @@ -47,10 +47,10 @@ public static long BitPosDriver(byte* input, int inputLen, long startOffset, lon if (startByteIndex >= inputLen) // If startOffset greater that valLen always bitpos -1 return -1; - if (startByteIndex > endByteIndex) // If start offset beyond endOffset return 0 + if (startByteIndex > endByteIndex) // If start offset beyond endOffset return -1 return -1; - endOffset = endByteIndex >= inputLen ? bitLen : endOffset; + endOffset = endByteIndex >= inputLen ? bitLen - 1 : endOffset; // BIT search return BitPosBitSearch(input, inputLen, startOffset, endOffset, searchFor); diff --git a/test/standalone/Garnet.test.complexstring/GarnetBitmapTests.cs b/test/standalone/Garnet.test.complexstring/GarnetBitmapTests.cs index 98409fc5555..e10cc46419e 100644 --- a/test/standalone/Garnet.test.complexstring/GarnetBitmapTests.cs +++ b/test/standalone/Garnet.test.complexstring/GarnetBitmapTests.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Linq; using System.Numerics.Tensors; +using System.Runtime.InteropServices; using System.Threading.Tasks; using Garnet.common; using Garnet.server; @@ -2408,6 +2409,76 @@ public void BitmapBitPosLongOffsetBoundaryValidationTest() ClassicAssert.AreEqual(-1, pos); } + /// + /// Regression test for an off-by-one clamp in BitPosDriver's BYTE-mode path: an explicit end + /// offset greater than or equal to the value's length was clamped to inputLen instead of + /// inputLen - 1, allowing BitPosByteSearch to read one byte past the end of the value + /// when no matching bit exists within the real data (mirrors the fix applied to BitCountDriver + /// in #1138, which was never mirrored to BITPOS). + /// + /// Calls BitmapManager.BitPosDriver directly against an unmanaged buffer with a known "canary" + /// byte placed immediately past the declared value length, so the out-of-bounds read is + /// deterministically observable: a matching bit hides in the canary byte, so the buggy driver + /// reports a real (non -1) position instead of -1, while the fixed driver never looks at it. + /// A plain end-to-end BITPOS call would rely on whatever bytes happen to follow the value on + /// the managed heap, which is not reliable enough to catch a one-byte overread. + /// + [Test] + [Category("BITPOS")] + public unsafe void BitmapBitPosOutOfBoundsEndOffsetByteModeTest() + { + const int valueLen = 8; + var buf = (byte*)NativeMemory.Alloc(valueLen + 1); + try + { + // Real value: no set bits anywhere within its declared bounds. + for (var i = 0; i < valueLen; i++) + buf[i] = 0x00; + + // Canary byte, one past the declared length. All bits set, so if the driver reads it + // while searching for a set bit, it will incorrectly report a position inside it. + buf[valueLen] = 0xFF; + + var pos = BitmapManager.BitPosDriver(buf, valueLen, startOffset: 0, endOffset: 1_000_000, searchFor: 1, offsetType: 0x0); + + ClassicAssert.AreEqual(-1, pos, "BITPOS BYTE mode must not read past the end of the value"); + } + finally + { + NativeMemory.Free(buf); + } + } + + /// + /// Same regression as , but for + /// BitPosDriver's BIT-mode path, whose equivalent clamp had the same off-by-one + /// (bitLen instead of bitLen - 1). + /// + [Test] + [Category("BITPOS")] + public unsafe void BitmapBitPosOutOfBoundsEndOffsetBitModeTest() + { + const int valueLen = 3; + var buf = (byte*)NativeMemory.Alloc(valueLen + 1); + try + { + // Real value: no set bits anywhere within its declared bounds (24 valid bit positions). + for (var i = 0; i < valueLen; i++) + buf[i] = 0x00; + + // Canary byte, one past the declared length. All bits set. + buf[valueLen] = 0xFF; + + var pos = BitmapManager.BitPosDriver(buf, valueLen, startOffset: 0, endOffset: 1_000_000, searchFor: 1, offsetType: 0x1); + + ClassicAssert.AreEqual(-1, pos, "BITPOS BIT mode must not read past the end of the value"); + } + finally + { + NativeMemory.Free(buf); + } + } + [Test] [Category("BITPOS")] public void BitmapBitPosBitModifierRequiresStartAndEndTest()