LMS/HSS private key decode does not validate d, q, maxQ or the tree cache, so a corrupted stored key is accepted and then throws unchecked exceptions or signs incorrectly

[Arpan0995]

Summary

The LMS/HSS private key decoder validates almost none of the fields it reads. A stored key whose d, q, maxQ or cached tree nodes have been altered is accepted by HSSPrivateKeyParameters.getInstance / LMSPrivateKeyParameters.getInstance (and by KeyFactory.generatePrivate on top of them), and the bad values then surface as an unchecked exception out of the signing call, or as a valid-looking key that signs incorrectly.

This is inconsistent with two neighbours that do validate:

• HSSPublicKeyParameters.getInstance already rejects an out-of-range level count with if (L < 1 || L > 8) // RFC 8554, Section 6. (HSSPublicKeyParameters.java:44). The private key side has no equivalent for d.
• The XMSS private key decoder validates its index (XMSSUtil.isIndexValid) and its whole BDS traversal state (BDS.validate(params, expectedIndex)) on reload. LMS has no counterpart.

A stateful HBS private key is rewritten to storage on every signature, so partial writes, bit rot and buggy storage layers are ordinary non-adversarial failure modes. The desired behaviour for a stateful key is to detect a corrupt state and refuse, which is what the XMSS side does.

Environment

Verified against origin/main 36f2d9fad, built from source, and against the current 1.86-SNAPSHOT beta sources (downloads.bouncycastle.org/betas, last modified 2026-08-28), which are byte-identical to origin/main for the two files below. Line numbers are from origin/main. The classes are the promoted org.bouncycastle.crypto.params copies; the deprecated org.bouncycastle.pqc.crypto.lms copies carry the same code.

The reproductions below use the public org.bouncycastle.crypto API, which is exactly what the LMS KeyFactory/Signature provider delegates to (LMSKeyFactorySpi -> BCLMSPrivateKey -> PrivateKeyFactory.createKey -> HSSPrivateKeyParameters.getInstance). The same acceptance happens through the JCA path: KeyFactory.getInstance("LMS", "BC").generatePrivate(new PKCS8EncodedKeySpec(corruptedEncoding)) returns a key without InvalidKeySpecException, and that key then signs.

1. HSS level count d is unvalidated, giving an unchecked IndexOutOfBoundsException

HSSPrivateKeyParameters.getInstance reads d, index and maxIndex with no range check (HSSPrivateKeyParameters.java:103-105); the only guard in that block is on the version. d = 0 and d = -1 are accepted, and the empty key list then fails inside rangeTestKeys:

decode of a d = 0 key                : ACCEPTED (no exception)
then generateLMSContext()            : throws
  java.lang.IndexOutOfBoundsException: Index -1 out of bounds for length 0
    at org.bouncycastle.crypto.params.HSSPrivateKeyParameters.rangeTestKeys(HSSPrivateKeyParameters.java:439)
    at org.bouncycastle.crypto.params.HSSPrivateKeyParameters.generateLMSContext(HSSPrivateKeyParameters.java:608)

KeyFactory.generatePrivate documents InvalidKeySpecException and Signature.update/sign document SignatureException; callers get neither.

Suggested fix: reject d < 1 || d > 8 at decode, reusing the wording already in HSSPublicKeyParameters:44-46, and reject index < 0 || maxIndex < 0 || index > maxIndex.

2. LMS one-time index q and maxQ are unvalidated, so a key signs outside its own tree

LMSPrivateKeyParameters.readCoreKey validates the master secret length carefully (l < 0 at line 206, l > available() at line 210) but reads q and maxQ with no check at all (lines 203-204). q < 0, q = Integer.MIN_VALUE, and q past the tree size are all accepted at decode; q = -1 then throws NullPointerException and q = Integer.MIN_VALUE throws ArrayIndexOutOfBoundsException out of the signing call. With q = 33 for h = 5 (so 2^h = 32) and maxQ raised past it, the only guard in generateLMSContext (q >= maxQ) passes, the key reports getIndex() = 33, and it signs. The signature does not verify under the real public key:

decode of an LMS key with q = 33 (> 2^h = 32), maxQ = 1000 : ACCEPTED, getIndex() = 33
sign                                                        : ok, 8684 bytes
verify under the real public key                           : false

Because q selects the LM-OTS leaf, this is a signature made with a one-time key outside the key's own Merkle tree, produced silently.

Suggested fix: reject q < 0, maxQ < 0, maxQ > (1 << parameter.getH()), and q > maxQ. The exhaustion guard rangeTestKeys at line 439 also uses an exact equality getIndex() == 1 << h; a q set above 1 << h steps over it, so that test should be >=.

3. The tree cache node values are unvalidated, and the authoritative public key next to them is discarded

The tree cache added in #2365 stores the top Merkle nodes so the first signature after a reload does not rebuild the tree. readTreeCache validates the node count and the byte length (LMSPrivateKeyParameters.java:224-232) but never the node values; primeTreeCache installs them verbatim. The cached nodes are a deterministic function of I, the master secret and the parameters, so they are fully checkable and are not checked.

A single-bit sweep of all 63 cached nodes of an h = 5 key, one bit each:

1 node  (node 1, the root) silently changes the public key that getPublicKey() reports:
   true reported root  : ...c6093e189dff91bc4a736cb3...
   after 1-bit flip     : ...c6093e189dff91bc4b736cb3...   (signing still works, verifies under the true pk)
5 nodes  produce a signature that does not verify

The corruption of node 1 also survives a re-encode (getEncoded writes the poisoned nodes back), so once it is in the stored state it stays there.

getInstance(privEnc, pubEnc) already parses the authoritative public key into the field at HSSPrivateKeyParameters.java:84, but that field is never read anywhere in the class (it is written at line 84 and there are no reads; getPublicKey() at line 414 recomputes from getRootKey().getPublicKey(), i.e. from the cache). So the correct value is available at decode time and is thrown away.

Suggested fix: recompute the cached interior nodes from their cached children and reject on mismatch. For h = 5 this is 31 hashes, microseconds against the full tree rebuild the cache exists to avoid, and it catches every single-node corruption because every cached node except node 1 has a cached parent and node 1 is recomputed from nodes 2 and 3. Alternatively, use the parsed publicKey field to check the reconstructed public key, or delete the field.

Severity

None of these is a forgery or a verification bypass, and an actor who can rewrite the stored private key already holds the master secret, so there is no confidentiality or forgery gain. The weight is that a stateful key silently accepts a corrupt state rather than refusing it, and that decode/sign throw unchecked IndexOutOfBoundsException, NullPointerException and ArrayIndexOutOfBoundsException where the JCA contract is InvalidKeySpecException / SignatureException. This is the same class already accepted as a defect for malformed ML-DSA input, and it is confined to the LMS/HSS path; XMSS and the HSS public key both validate. Default builds are affected, no special flags.

I am happy to send a pull request if that is useful.

[dghgit]

Thanks for the report, we agree this is a hardening worth addressing. In any case, this one caused quite an adventure and we found a couple of other issues related to it. These are now fixed in the latest beta on https://downloads.bounycastle.org/betas

[Arpan0995]

Thanks, good to see this dealt with, and nice that the digging turned up the related decoder issues too.