Fix out-of-bounds read in hss_validate_signature_init - #2507
Conversation
hss_validate_signature_init enforced only a 4-byte minimum before parsing a 12-byte header and, on the single-level path, reading the LM-OTS randomizer C at offset 8 of the remaining signature. A truncated signature therefore read past the end of the caller's buffer. Bound the header and re-check the remaining length against 8+n once the LM-OTS parameter set is known. Signed-off-by: Ramya Eliger <ramya@digiscrypt.com>
|
@Ramya-9353, apologies for the delay in responding. This appears to be a duplicated issue and I had been working on the duplicate. The solutions are basically identical. Here are the diffs. If you would like to incorporate them... git diff +/* RFC 8554 u32 field width in serialized HSS/LMS structures /
/* This is the length of our internal seed values / diff --git a/src/sig_stfl/lms/external/hss_verify_inc.c b/src/sig_stfl/lms/external/hss_verify_inc.c
@@ -116,8 +116,9 @@ bool hss_validate_signature_init(
|
Replace the literal 12-byte header bound with HSS_SIG_INC_HEADER_LEN, derived from the u32 field widths of u32(levels-1) || u32(q) || u32(lm_ots), and move the bound into is_hss_public_key() so the function that reads offsets 0..11 checks the length itself. Signed-off-by: Ramya Eliger <ramya@digiscrypt.com>
|
No worries. Incorporated in a12e152. One adaptation: Rebuilt with ASAN and ran test_sig_stfl for LMS_SHA256_H5_W2, H5_W8, H10_W2 and the two-level H5_W8_H5_W8 and H10_W4_H5_W8 sets, all clean; XMSS unaffected. If your branch is further along, happy for that one to land instead and I'll close this. |
The test exercised only lengths 4 and 12, so neither bound the parser now enforces was tested at its edge. Add 11, the largest length below the 12-byte incremental header; 39, 40 and 43, with 43 the largest length that leaves fewer than 8 + n bytes for q and the LM-OTS randomizer C; and 44, the shortest signature that clears both bounds and therefore reaches lm_validate_signature, which must reject it on length without an over-read. Derive the loop bound from the array so the two cannot drift apart again. Verified on arm64-Darwin with -DUSE_SANITIZER=Address, LMS enabled and stateful key/signature generation off: each of 4, 11, 12, 39, 40 and 43 reports a heap over-read against unpatched main through both OQS_SIG_STFL_verify and the exported OQS_SIG_STFL_alg_lms_verify, 44 is rejected cleanly on both, and the extended test passes on this branch for single- and two-level parameter sets with all 18 LMS KATs unchanged. Prepared with the assistance of Claude Code (Anthropic): it chose the boundary lengths, ran the sanitizer and KAT checks recorded above, and drafted this message. It did not alter the fix itself. Signed-off-by: Douglas Stebila <dstebila@uwaterloo.ca>
|
I've pushed one commit (8d3d4f5) extending the truncation set in the regression test from {4, 12} to {4, 11, 12, 39, 40, 43, 44}, and deriving the loop bound from the array so the two can't drift apart. Both existing lengths sit inside the failing range but neither is at a boundary. 11 is the largest length below the 12-byte header; 43 is the largest that leaves fewer than 8 + n bytes for q and C; and 44 is the shortest input that clears both new bounds, which matters because it's the first length to reach lm_validate_signature and must be rejected by that function's own length checks. Verified locally on arm64-Darwin, Debug + ASan, LMS enabled with stateful key/signature generation off: 4, 11, 12, 39, 40 and 43 each report a heap over-read against unpatched main through both OQS_SIG_STFL_verify and the exported OQS_SIG_STFL_alg_lms_verify; 44 is rejected cleanly on both; the extended test passes on this branch for single- and two-level parameter sets; all 18 LMS KATs pass; astyle clean. Per CONTRIBUTING.md: this commit was prepared with the assistance of Claude Code. The AI selected the boundary lengths, ran the builds and the sanitizer and KAT checks recorded above, and drafted the commit message and this comment. It did not modify the fix. |
3 participants
Repro: call
OQS_SIG_STFL_verifyfor any LMS parameter set with a 12-byte signature buffer whose header agrees with the public key (u32(0) || u32(q) || u32(lm_ots)). ASAN reports a 32-byte heap over-read athss_verify_inc.c:215; a 4-byte buffer over-reads 8 bytes atis_hss_public_key. Both are reachable in three calls from the public API with fully attacker-controlled bytes, in any build, since LMS verify is never compiled out.Cause:
hss_validate_signature_initestablishes a 4-byte minimum, then parses a 12-byte header and, on the single-level path where the bounds-checkedlevels > 1loop is skipped entirely, readsqand the n-byte LM-OTS randomizerCwithout revalidating what remains.Fix: require the 12 bytes the header actually occupies, and re-check the remaining length against
8 + nonce the LM-OTS parameter set is known. Both bounds sit well below the smallest well-formed LMS signature, so valid inputs verify exactly as before; the LMS and HSS KATs are unchanged.The equivalent guard already exists on the XMSS side (GHSA-wf7v-fhxj-73m2), applied there in the per-variant verify wrapper. LMS installs the generic verify with no length check at all, and
tests/fuzz_test_sig_stfl_lms.calways passessig->length_signature, so the short-signature case was never exercised. The added regression test allocates each truncated buffer at exactly the length handed to verify, so the sanitiser jobs observe any over-read.