[security] rist_decode accepts non-canonical Ristretto255 encodings at 071b0e9

[bhu1tyagi]

rist_decode in include/pvac/crypto/ristretto255.hpp accepts non-canonical
byte encodings of a Ristretto255 point. Two different 32 byte strings that
differ only in bit 255 decode to the same group element and are both accepted.
RFC 9496 4.3.1 requires a decoder to reject any encoding whose integer is at or
above p, or whose high bit is set. This decoder does neither range check.

Not a key recovery break for the HFHE bounty (the vulnerable path is not in
secret.ct), but it breaks the canonical-encoding property ristretto is meant to
guarantee, which a few places in this repo rely on.

Root cause

fe_frombytes masks off the top bit instead of rejecting it:

// ~line 149
uint64_t h4 = ... | ((uint64_t)s[31] & 0x7f) << 44;   // bit 255 dropped, not rejected
return fe_reduce(Fe25519{{ ... h4 & FE_MASK51 }});     // reduced mod p, no range check

and rist_decode never checks canonicity of the input bytes:

// ~line 639
inline bool rist_decode(ExtPoint& P, const RistrettoPoint& bytes) {
    Fe25519 s = fe_frombytes(bytes.data());
    if (fe_is_negative(s)) return false;   // only check on the input
    ...

Because bit 255 is masked and the field element is reduced mod p = 2^255 - 19,
both the high-bit-set encodings and the encodings whose little endian integer is
in [p, 2^255) are silently normalized and accepted.

Reproduction

Self contained, uses only this header. Build:
g++ -std=c++17 -O2 -I pvac_hfhe_cpp/include -o rcr rist_canon_repro.cpp

#include <pvac/pvac.hpp>
#include <cstdio>
using namespace pvac;
static void hex(const char* l, const uint8_t* b){ printf("%s", l); for(int i=0;i<32;i++) printf("%02x", b[i]); printf("\n"); }
int main(){
    RistrettoPoint H = rist_H();
    ExtPoint P0; bool ok0 = rist_decode(P0, H);
    printf("canonical H decodes: %d\n", ok0);
    hex("H (canonical)     = ", H.data());
    RistrettoPoint H_hi = H; H_hi.data()[31] |= 0x80;   // set bit 255
    ExtPoint P1; bool ok1 = rist_decode(P1, H_hi);
    hex("H with bit255 set = ", H_hi.data());
    printf("non-canonical accepted: %d, decodes to same point: %d\n",
           ok1, ok1 && (rist_encode(P1) == rist_encode(P0)));
    return 0;
}

Output:

canonical H decodes: 1
H (canonical)     = c8bbb3f6edabb3bd7beb0e03c1630bcbffc77d27defbc1cce416d6f7f9408961
H with bit255 set = c8bbb3f6edabb3bd7beb0e03c1630bcbffc77d27defbc1cce416d6f7f94089e1
non-canonical accepted: 1, decodes to same point: 1

Impact

Ristretto guarantees unique canonical encodings, so higher level code can hash
or compare point bytes safely. This defect breaks that:

• fold_sig_digest in recrypt_fold.hpp hashes PC bytes into a layer identity
  used for dedup. A malleable encoding lets the same commitment map to different
  fold signatures, or two "different" layers be equal, in any flow where a point
  encoding is attacker influenced.
• Any Fiat-Shamir transcript or equality check built on serialized points is
  affected.

Suggested fix

Enforce canonical decoding in rist_decode:

if (bytes.data()[31] & 0x80) return false;               // reject high bit
Fe25519 s = fe_frombytes(bytes.data());
uint8_t canon[32]; fe_tobytes(canon, s);
if (memcmp(canon, bytes.data(), 32) != 0) return false;  // must be canonical
if (fe_is_negative(s)) return false;
...

Note (already tracked, not part of this report)

While confirming the above I noticed the bounty is pinned to 071b0e9, where
hidden_coeff_stmt_digest in recrypt_hidden_coeff.hpp still hashes the secret
alpha vector under a public-only salt (same shape as the old R_com oracle).
PR #499 fixes this, but anything built at the pinned commit predates the fix.
It does not appear reachable from the static secret.ct, so it does not break
the bounty, but pinning the challenge past the #499 merge would be cleaner.