Skip to content

Harden arithmetic and crypto boundary checks across u64::shr, ilog2, u32clz, and Falcon mod_12289#2808

Open
huitseeker wants to merge 6 commits intoaudit-fixes-2026from
huisteeker/audit-fixes-2026/instructions
Open

Harden arithmetic and crypto boundary checks across u64::shr, ilog2, u32clz, and Falcon mod_12289#2808
huitseeker wants to merge 6 commits intoaudit-fixes-2026from
huisteeker/audit-fixes-2026/instructions

Conversation

@huitseeker
Copy link
Contributor

@huitseeker huitseeker commented Mar 9, 2026

  • Tightens boundary/overflow verification logic in four sensitive paths:

    1. u64::shr: fixes behavior for shifts >= 32 by correctly zeroing the high limb in that branch.
    2. ilog2: replaces half-word-style validation with explicit full-width lower/upper bound checks (2^ilog2 <= n < 2^(ilog2+1), with ilog2 == 63 special-case handling).
    3. u32clz: enforces exact zero boundary (n == 0 iff clz == 32) and strengthens mask consistency checks.
    4. Falcon mod_12289: converts previously dropped overflow flags into explicit assertions for quotient/addition overflow safety.
  • Adds focused regression/security-style tests for each fix:

    1. u64::shr edge cases at 32, >32, and boundary sweeps.
    2. ilog2 forged advice and boundary-claim rejection/acceptance tests.
    3. u32clz zero/non-zero boundary correctness tests.
    4. Falcon malicious advice/overflow rejection tests.
  • Updates cycle-count documentation/comments where instruction costs changed (u64::shr, u32clz, ilog2).

Per-commit review recommended

@huitseeker huitseeker changed the base branch from next to audit-fixes-2026-speculatively-next March 9, 2026 18:31
@huitseeker huitseeker force-pushed the huisteeker/audit-fixes-2026/instructions branch from 5f15d3e to f29b4f0 Compare March 9, 2026 18:47
#[test]
fn verify_clz_accepts_zero_with_clz_32() {
run_verify_clz_gadget(0, 32).expect("gadget should accept valid zero boundary witness");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these should be MASM-based tests, since the ultimate goal is to test the regression with the assembler-generated code for u32clz. Instead, we're testing here the hardcoded run_verify_clz_gadget(), which might get out of date with what the assembler is doing.

Copy link
Contributor Author

@huitseeker huitseeker Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added MASM tests, but MASM tests validate observed output under normal system behavior, not
explicit rejection of malicious/wrong CLZ witnesses, so I've kept those.

@huitseeker huitseeker force-pushed the huisteeker/audit-fixes-2026/instructions branch from 04c3303 to a3ab057 Compare March 10, 2026 21:42
@huitseeker huitseeker changed the base branch from audit-fixes-2026-speculatively-next to audit-fixes-2026 March 11, 2026 16:08
@huitseeker huitseeker force-pushed the huisteeker/audit-fixes-2026/instructions branch 2 times, most recently from 9b51eaf to b20c19d Compare March 11, 2026 16:15
Copy link
Contributor

@plafer plafer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@huitseeker huitseeker force-pushed the huisteeker/audit-fixes-2026/instructions branch from b20c19d to 0c28914 Compare March 11, 2026 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants