bqfc_decompr: reject decompressed forms where |b| > a (fixes b0 malleability)

[richardkiss]

Fixes #334.

Problem

bqfc_verify_canon performs a self-consistency check — encode(decode(X)) == X — rather than a uniqueness check — encode(reduce(decode(X))) == X.

For forms with g > 1, the b0 field can be inflated by any multiple of a' = a/g and still round-trip through bqfc_compr, because the xgcd_partial Euclidean path is unaffected and b0 = floor(|b|/a') picks up the inflated quotient.

Concretely (1024-bit discriminant, g=2, g_size=0): byte 99 (b0) accepts 64 distinct values — b0, b0+4, b0+8, … — instead of 1. DeserializeForm silently reduces the form via from_abd, so Wesolowski verification passes with the correct (reduced) y. The proof is malleable: the same mathematical VDF output has 64 valid serializations.

Fix

After bqfc_decompr computes (out_a, out_b), assert |out_b| <= out_a. A reduced binary quadratic form requires |b| <= a; if this is violated the encoding cannot be canonical.

The check is a single mpz_cmpabs call — no need to compute c or run the full Pulmark reduction.

if (mpz_cmpabs(out_b, out_a) > 0) {
    ret = -1;
    goto out;
}

Why this is the right place

The root cause is that bqfc_decompr can produce a non-reduced (out_a, out_b), and bqfc_verify_canon compares against the encoding of that non-reduced form rather than the reduced representative. Adding the bounds check here:

1. Costs one GMP comparison per deserialization.
2. Requires no changes to callers.
3. Fixes the issue at the correct semantic layer (decompr guarantees a reduced form or fails).

Regression test

Added RejectsInflatedB0Field to proof_deserialization_regression_test.cpp using a real Chia mainnet vector (block 309155, CC infusion-point VDF). The canonical b0=0x01 is accepted; b0=0x05 and b0=0x09 (same mod-4 residue, previously accepted) are now rejected.

Testing

Discovered via differential fuzzing of chia-vdf-verify (pure-Rust reimplementation) against chiavdf, where Rust's stricter is_reduced() check exposed the discrepancy. See the characterization in issue #334.

Made with Cursor

---

Note

Medium Risk
Touches consensus-adjacent proof/form deserialization logic and changes function signatures, so missed call-site updates or strictness toggling could cause verification incompatibilities or unexpected rejects.

Overview
Hardens BQFC form decoding against b0 malleability. bqfc_decompr/bqfc_deserialize now accept a strict flag and, when enabled, reject decoded forms where |b| > a (preventing multiple self-consistent serializations of the same class-group element).

Call sites are updated to plumb the new flag (keeping existing behavior lenient in hw_proof and defaulting DeserializeForm to non-strict for compatibility), and new regression tests (C++ and Python) cover canonical vs inflated b0 vectors. The Python module also exposes a low-level bqfc_deserialize helper returning signed big-endian (a,b) bytes for external verification/debugging.

^{Reviewed by Cursor Bugbot for commit 7e62470. Bugbot is set up for automated code reviews on this repo. Configure here.}

[Copilot]

Pull request overview

This PR tightens compressed quadratic-form deserialization to eliminate a known malleability vector in the b0 field, and adds a regression test using a real mainnet-derived vector.

Changes:

• Add an additional validity check during bqfc_decompr to reject certain non-canonical decodings.
• Add a regression test that mutates b0 (inflation by +4/+8) and asserts deserialization rejection.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/proof_deserialization_regression_test.cpp	Adds a regression test covering b0 inflation malleability using a fixed mainnet vector.
src/bqfc.c	Adds a post-decompression bound check intended to reject inflated b0 encodings that previously passed canonical verification.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The new non-canonical rejection only checks |b| > a, but in this codebase the canonical normalized range for reduced forms is -a < b <= a (i.e., b == -a should also be rejected). As written, an encoding that decodes to b == -a would pass this check and can still be normalized by later reduction logic, leaving a potential alternative encoding path. Consider extending the predicate to also fail when out_b == -out_a (or equivalently abs(b)==a && b<0).

Suggested change

	* Reject if |b| > a. For a reduced form, |b| <= a must hold. If b0 is
	* inflated (e.g. b0 = canonical_b0 + 4k for k != 0) the decoded b lands
	* outside this range even though bqfc_verify_canon would otherwise pass
	* (the self-consistency check encode(decode(X))==X is satisfied for any
	* b0 ≡ canonical_b0 mod (a/gcd(a,t))). Rejecting here makes the
	* canonical-check a proper uniqueness gate.
	*/
	if (mpz_cmpabs(out_b, out_a) > 0) {
	* Reject if |b| > a, or if b == -a. For a reduced form in this codebase,
	* the canonical range is -a < b <= a, so b == -a must also be rejected.
	* If b0 is inflated (e.g. b0 = canonical_b0 + 4k for k != 0) the decoded
	* b lands outside this range even though bqfc_verify_canon would
	* otherwise pass (the self-consistency check encode(decode(X))==X is
	* satisfied for any b0 ≡ canonical_b0 mod (a/gcd(a,t))). Rejecting here
	* makes the canonical-check a proper uniqueness gate.
	*/
	if (mpz_cmpabs(out_b, out_a) > 0 ||
	(mpz_cmpabs(out_b, out_a) == 0 && mpz_sgn(out_b) < 0)) {

[Copilot]

This test hard-codes the b0 byte offset as 99. Since b0 is the last byte for this vector (g_size=0), using BQFC_FORM_SIZE - 1 (or computing the offset from the format fields) would make the test resilient to future format/size changes and avoid the magic number.

[Copilot]

Same as above: this second mutation also uses the hard-coded index 99 for b0. Prefer BQFC_FORM_SIZE - 1 (or a derived offset) to avoid a brittle magic number.