From 7ecbd7385a3e9c8f53bbfb3817e987f7fcbdb896 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 31 Jul 2026 15:31:14 +0200 Subject: [PATCH] ed448: remove unreachable privKeySet operand in wc_ed448_check_key Premise: ed448.c:1517 `if ((ret == 0) && key->privKeySet) { ... }` -- the sibling arm of the if/else this condition belongs to. Claim: ed448.c:1524 `else if ((ret == 0) && (!key->privKeySet))` -- the `!key->privKeySet` operand; and, as a direct consequence, the nested `if (ret == 0)` that opened the arm's body. Proof: entering the else arm means the sibling condition was false, i.e. (ret != 0) || (!privKeySet). && short-circuits, so the else arm's own `key->privKeySet` read happens only after `ret == 0` evaluated true -- and with ret == 0 the sibling's falsity forces !privKeySet. The operand is therefore invariantly true. privKeySet is a plain bitfield, not volatile, and no statement runs between the two conditions. The nested `if (ret == 0)` follows from the same short-circuit: the arm is entered only with ret == 0 and nothing precedes the nested test. Scope: wc_ed448_check_key contains no #if/#ifdef, so this holds in every configuration that compiles ed448.c. Evidence: llvm-cov MC/DC records this condition's pair as uncovered (reports/ed448/GAPS.md row 1503:14:1503:46:1, pre-drift numbering). Reviewed with `git diff -w`: apart from the two removed lines the change is pure de-indentation. ed448.c is inside the FIPS module boundary. Released FIPS flavours pin ed448.c to a tag and are unaffected; fips-dev/fips-ready build from master and recompute the in-core hash. Compiler cross-check: gcc -O2 emits byte-identical code for this file before and after this commit -- the optimiser had already folded the removed condition, independently confirming it was dead. --- wolfcrypt/src/ed448.c | 50 +++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/wolfcrypt/src/ed448.c b/wolfcrypt/src/ed448.c index 08374ba446..73e0c0bcc1 100644 --- a/wolfcrypt/src/ed448.c +++ b/wolfcrypt/src/ed448.c @@ -1521,41 +1521,39 @@ int wc_ed448_check_key(ed448_key* key) } } /* No private key, check Y is valid. */ - else if ((ret == 0) && (!key->privKeySet)) { + else if (ret == 0) { /* Verify that xQ and yQ are integers in the interval [0, p - 1]. * Only have yQ so check that ordinate. * p = 2^448-2^224-1 = 0xff..fe..ff */ - if (ret == 0) { - int i; - ret = PUBLIC_KEY_E; + int i; + ret = PUBLIC_KEY_E; - /* Check top part before 0xFE. */ - for (i = ED448_PUB_KEY_SIZE - 1; i > ED448_PUB_KEY_SIZE/2; i--) { - if (key->p[i] < 0xff) { - ret = 0; - break; - } + /* Check top part before 0xFE. */ + for (i = ED448_PUB_KEY_SIZE - 1; i > ED448_PUB_KEY_SIZE/2; i--) { + if (key->p[i] < 0xff) { + ret = 0; + break; } - if (ret == WC_NO_ERR_TRACE(PUBLIC_KEY_E)) { - /* Check against 0xFE. */ - if (key->p[ED448_PUB_KEY_SIZE/2] < 0xfe) { - ret = 0; - } - else if (key->p[ED448_PUB_KEY_SIZE/2] == 0xfe) { - /* Check bottom part before last byte. */ - for (i = ED448_PUB_KEY_SIZE/2 - 1; i > 0; i--) { - if (key->p[i] != 0xff) { - ret = 0; - break; - } - } - /* Check last byte. */ - if ((ret == WC_NO_ERR_TRACE(PUBLIC_KEY_E)) && - (key->p[0] < 0xff)) { + } + if (ret == WC_NO_ERR_TRACE(PUBLIC_KEY_E)) { + /* Check against 0xFE. */ + if (key->p[ED448_PUB_KEY_SIZE/2] < 0xfe) { + ret = 0; + } + else if (key->p[ED448_PUB_KEY_SIZE/2] == 0xfe) { + /* Check bottom part before last byte. */ + for (i = ED448_PUB_KEY_SIZE/2 - 1; i > 0; i--) { + if (key->p[i] != 0xff) { ret = 0; + break; } } + /* Check last byte. */ + if ((ret == WC_NO_ERR_TRACE(PUBLIC_KEY_E)) && + (key->p[0] < 0xff)) { + ret = 0; + } } }