Merged PR 15120207: Fix heap buffer overflow in SymCryptXmssSign when height >= 32

Fix for MSRC 111294: SymCryptXmssSign function - Heap overflow via 64->32-bit leaf-count truncation

This PR fixes heap buffer overflow in `SymCryptXmssSign` when signing with XMSS^MT parameter sets with a height of 32 or greater. Note that XMSS(^MT) signing is only approved by FIPS and Microsoft SDL when performed inside an HSM; XMSS(^MT) signing in SymCrypt is provided only for testing and validation purposes and should not be used in production software.

- Update callers of `SymCryptHbsSizeofScratchBytesForIncrementalTreehash` to consistently use `1ULL << pParams->nLayerHeight` for `nLeaves` - since XMSS^MT operations are done layer-by-layer, scratch space should only allocate space for one layer (consistent with how `SymCryptXmssVerifyInternal` already worked)
- Add regression test with custom parameter set to exercise 32-deep tree

Validated:
- Local unit tests
- Validated POC on VM: ASAN crashes without fix, does not crash with fix

Thanks to [Federico Ponzi](https://fponzi.me) for finding this.

Related work items: #61560104

Author: mlindgren

CHANGELOG.md

@@ -5,6 +5,8 @@ prior to the creation of a new release, based on the changes contained in that r
 # Version 103.11.0
 
 - Add Composite ML-KEM implementation
+- Fix heap buffer overflow in `SymCryptXmssSign` when signing with XMSS^MT parameter sets with a height of 32 or greater
+  - Thanks to [Federico Ponzi](https://fponzi.me) for finding this
 
 # Version 103.10.2
 

lib/sc_lib.h

@@ -5096,3 +5096,39 @@ SymCryptCountLeadingZeros32( UINT32 value )
 
     return (UINT32)zeros;
 }
+
+FORCEINLINE
+UINT32
+SymCryptCountLeadingZeros64( UINT64 value )
+{
+    unsigned long zeros = 0;
+
+    if(value == 0)
+    {
+        return 64;
+    }
+
+#if SYMCRYPT_MS_VC && (SYMCRYPT_CPU_AMD64 | SYMCRYPT_CPU_ARM64)
+    _BitScanReverse64(&zeros, value);
+    zeros = 63 - zeros;
+#elif SYMCRYPT_MS_VC && (SYMCRYPT_CPU_X86 | SYMCRYPT_CPU_ARM)
+    if( (value >> 32) == 0 )
+    {
+        _BitScanReverse(&zeros, (UINT32)value);
+        zeros = 63 - zeros;
+    } else {
+        _BitScanReverse(&zeros, (UINT32)(value >> 32));
+        zeros = 31 - zeros;
+    }
+#elif SYMCRYPT_GNUC
+    zeros = __builtin_clzll(value);
+#else
+    while( (value & 0x8000000000000000) == 0 )
+    {
+        zeros++;
+        value <<= 1;
+    }
+#endif
+
+    return (UINT32)zeros;
+}

lib/xmss.c

@@ -417,8 +417,8 @@ SymCryptXmssSetParams(
         goto cleanup;
     }
 
-    // Layer height (tree height of one layer) can be at most 32
-    if ((nTotalTreeHeight / nLayers) > 32)
+    // Layer height (tree height of one layer) can be at most 31
+    if ((nTotalTreeHeight / nLayers) > 31)
     {
         scError = SYMCRYPT_INVALID_ARGUMENT;
         goto cleanup;
@@ -720,9 +720,11 @@ SymCryptHbsSizeofScratchBytesForIncrementalTreehash(
     UINT32  cbNode,
     UINT32  nLeaves)
 {
-    SIZE_T nodeSize = (cbNode + 2 * sizeof(UINT32)) + sizeof(SYMCRYPT_INCREMENTAL_TREEHASH);
+    SIZE_T nodeSize = cbNode + 2 * sizeof(UINT32);
+    SIZE_T result = (sizeof(SYMCRYPT_INCREMENTAL_TREEHASH) - sizeof(SYMCRYPT_TREEHASH_NODE));
 
-    return nodeSize * SymCryptHbsIncrementalTreehashStackDepth(nLeaves);
+    result += nodeSize * SymCryptHbsIncrementalTreehashStackDepth(nLeaves);
+    return result;
 }
 
 
@@ -1015,8 +1017,8 @@ SymCryptXmssComputeSubtreeRoot(
     PSYMCRYPT_TREEHASH_NODE pNode = NULL;
     SYMCRYPT_XMSS_INCREMENTAL_TREEHASH_CONTEXT ctxIncHash;
 
-    // uLeaf must be a multiple of 2^uHeight
-    SYMCRYPT_ASSERT((uLeaf & ((1UL << uHeight) - 1)) == 0);
+    SYMCRYPT_ASSERT((uLeaf & ((1UL << uHeight) - 1)) == 0); // uLeaf must be a multiple of 2^uHeight
+    SYMCRYPT_ASSERT(pParams->nLayerHeight < 32); // Ensure nLeaves fits in 32 bits
 
     SIZE_T cbScratchTree = SymCryptHbsSizeofScratchBytesForIncrementalTreehash(pParams->cbHashOutput, 1ULL << pParams->nLayerHeight);
     SIZE_T cbScratchLtree = SymCryptHbsSizeofScratchBytesForIncrementalTreehash(pParams->cbHashOutput, pParams->len);
@@ -1078,6 +1080,8 @@ SymCryptXmssComputePublicRoot(
     SIZE_T cbScratch = 0;
     XMSS_ADRS adrs;
 
+    SYMCRYPT_ASSERT(pParams->nLayerHeight < 32); // Ensure nLeaves fits in 32 bits
+
     if (pbRoot == NULL || cbRoot != pParams->cbHashOutput ||
         pbSeed == NULL || cbSeed != pParams->cbHashOutput ||
         pbSkXmss == NULL || cbSkXmss != pParams->cbHashOutput)
@@ -1751,6 +1755,8 @@ SymCryptXmssVerifyInternal(
 
     SYMCRYPT_CHECK_MAGIC(pKey);
 
+    SYMCRYPT_ASSERT(pParams->nLayerHeight < 32); // Ensure nLeaves fits in 32 bits
+
     if (flags != 0 ||
         pbSignature == NULL ||
         cbSignature != SymCryptXmssSizeofSignatureFromParams(pParams) ||
@@ -1761,7 +1767,7 @@ SymCryptXmssVerifyInternal(
     }
 
     cbScratch += SymCryptHbsSizeofScratchBytesForIncrementalTreehash(pParams->cbHashOutput, pParams->len);
-    cbScratch += SymCryptHbsSizeofScratchBytesForIncrementalTreehash(pParams->cbHashOutput, 1ULL << (pParams->nTotalTreeHeight / pParams->nLayers));
+    cbScratch += SymCryptHbsSizeofScratchBytesForIncrementalTreehash(pParams->cbHashOutput, 1ULL << pParams->nLayerHeight);
 
     pbScratch = (PBYTE)SymCryptCallbackAlloc(cbScratch);
 
@@ -2037,9 +2043,10 @@ SymCryptXmssSign(
     UINT32 uLeaf;
     const UINT64 LeafMask = (1ULL << pParams->nLayerHeight) - 1;
 
-
     SYMCRYPT_CHECK_MAGIC(pKey);
 
+    SYMCRYPT_ASSERT(pParams->nLayerHeight < 32); // Ensure nLeaves fits in 32 bits
+
     if (flags != 0 ||
         pbSignature == NULL ||
         cbSignature != SymCryptXmssSizeofSignatureFromParams(pParams) ||
@@ -2050,7 +2057,7 @@ SymCryptXmssSign(
     }
 
     cbScratch += SymCryptHbsSizeofScratchBytesForIncrementalTreehash(pParams->cbHashOutput, pParams->len);        // Ltree hashing
-    cbScratch += SymCryptHbsSizeofScratchBytesForIncrementalTreehash(pParams->cbHashOutput, 1ULL << pParams->nTotalTreeHeight);  // Merkle-tree hashing
+    cbScratch += SymCryptHbsSizeofScratchBytesForIncrementalTreehash(pParams->cbHashOutput, 1ULL << pParams->nLayerHeight);  // Merkle-tree hashing
 
     pbScratch = (PBYTE)SymCryptCallbackAlloc(cbScratch);