Harden RESTORE validation and block malformed HLL payload misuse in PF operations

[vazois]

Summary

This PR fixes RESTORE misuse paths that allowed tampered dump payloads to be accepted and later used by HyperLogLog operations, including when checksum validation is explicitly skipped.

Changes

• Fixed RESTORE checksum gate in libs/server/Resp/KeyAdminCommands.cs to validate CRC by default (!SkipRDBRestoreChecksumValidation).
• Hardened HLL value validation in libs/server/Resp/HyperLogLog/HyperLogLog.cs:
  • Corrected dense/sparse length validation logic.
  • Enforced sparse payload bound (SparseRLESize <= allocated sparse payload).
  • Added full sparse stream semantic validation (IsValidSparseStream) to verify:
    • opcode validity,
    • non-zero register value bounds,
    • exact register-space coverage (no over/under-run).
• Added defense-in-depth PF-path checks in libs/server/Storage/Session/MainStore/HyperLogLogOps.cs to reject malformed HLL blobs before PFCOUNT/PFMERGE processing.
• Extended test server factory in test/Garnet.test/TestUtils.cs with skipRDBRestoreChecksumValidation to exercise checksum-skip scenarios.
• Added/updated security regressions in test/Garnet.test/HyperLogLogTests.cs:
  • corrupted dump payload rejection on RESTORE,
  • zero-CRC payload rejection on RESTORE,
  • corrupted sparse-RLE payload rejection on RESTORE,
  • sparse stream validator rejection cases,
  • checksum-skip mode test asserting malformed restored HLL returns WRONGTYPE for PFCOUNT/PFADD/PFMERGE,
  • representation/length parsing coverage for sparse/dense and 6/14/32-bit parser branches.

[Copilot]

Pull request overview

This PR strengthens Garnet’s handling of RESTORE payloads and HyperLogLog (PF*) operations to prevent malformed/tampered serialized values from being accepted and later misused during PF processing.

Changes:

• Enables RESTORE checksum validation by default (unless explicitly skipped via server option).
• Hardens HyperLogLog blob validation, including strict sparse-stream semantic checks.
• Adds/extends test utilities and regression tests to cover corrupted dumps and checksum-skip scenarios.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
libs/server/Resp/KeyAdminCommands.cs	Flips RESTORE checksum-validation gate to validate by default.
libs/server/Resp/HyperLogLog/HyperLogLog.cs	Tightens HLL length checks and validates sparse opcode stream semantics/coverage.
libs/server/Storage/Session/MainStore/HyperLogLogOps.cs	Adds PFCOUNT/PFMERGE defense-in-depth validation before processing blobs.
test/Garnet.test/TestUtils.cs	Extends test server factory to optionally skip RESTORE checksum validation.
test/Garnet.test/HyperLogLogTests.cs	Adds regression tests for corrupted dumps, sparse-stream validator cases, and checksum-skip behavior.

Comments suppressed due to low confidence (1)

libs/server/Resp/KeyAdminCommands.cs:81

• RESTORE CRC computation appears to hash the value-type byte as well (valueSpan.Slice(0, valueSpan.Length - 8)), but DUMP computes CRC starting after the value-type byte (payloadToHash slice starts after writing the type byte). With checksum validation now enabled by default, this mismatch would cause RESTORE to reject dumps produced by DUMP. Adjust the RESTORE hash input to skip the leading type byte so both commands use the same CRC coverage.

            if (!storeWrapper.serverOptions.SkipRDBRestoreChecksumValidation)
            {
                // crc is calculated over the encoded payload length, payload and the rdb version bytes
                // skip's the value type byte and crc64 bytes
                var calculatedCrc = new ReadOnlySpan<byte>(Crc64.Hash(valueSpan.Slice(0, valueSpan.Length - 8)));

                // skip's rdb version bytes
                var payloadCrc = footer[2..];

                if (calculatedCrc.SequenceCompareTo(payloadCrc) != 0)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.