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

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);

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);
         }
 
+        /// <summary>
+        /// 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 <c>inputLen</c> instead of
+        /// <c>inputLen - 1</c>, 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.
+        /// </summary>
+        [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);
+            }
+        }
+
+        /// <summary>
+        /// Same regression as <see cref="BitmapBitPosOutOfBoundsEndOffsetByteModeTest"/>, but for
+        /// BitPosDriver's BIT-mode path, whose equivalent clamp had the same off-by-one
+        /// (<c>bitLen</c> instead of <c>bitLen - 1</c>).
+        /// </summary>
+        [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()