Additional validation in crypto_get_uncompressed_pubkey
What the developer wrote
Authored by Salvatore Ingala
Additional validation in crypto_get_uncompressed_pubkey
Return with error if:
- x is too large
- x^3 + 7 is not a quadratic residue
What changed, and why it matters
This commit tightens checks in a Ledger Bitcoin app function that converts compressed public keys into full (x,y) public key points. Previously, the code did not verify that the supplied x-coordinate is a valid field element (less than the curve prime p) and did not confirm that the computed y really satisfies the secp256k1 curve equation. The patch adds both checks, rejecting malformed keys. Without these checks, a specially crafted compressed key could cause the function to produce a point that is not actually on the Bitcoin curve, which could then be used in subsequent cryptographic operations in unexpected ways.
Treat this as a security hardening fix and include it in the next release. Review all call sites of crypto_get_uncompressed_pubkey to ensure return codes are checked and that no downstream code assumes the output is always on-curve. Consider whether other apps (e.g., Bitcoin forks) share the same helper and need an equivalent fix.
Security signals we found
Missing input validation on externally supplied compressed public key x-coordinate
Off-curve public-key point could be produced before patch
Out-of-range x-coordinate accepted before patch
Modular square root of non-residue not detected before patch
Patch adds explicit field-membership and quadratic-residue checks
Evidence from the diff
In src/crypto.c, crypto_get_uncompressed_pubkey now: (1) rejects compressed keys where x >= secp256k1_p, and (2) after computing y = (x^3 + 7)^((p+1)/4) mod p, verifies y^2 mod p equals x^3 + 7 mod p. Previously the function only computed the modular square root and adjusted parity; it did not detect non-residue inputs or out-of-range x values. The change prevents off-curve or invalid-field public-key decompression. The function is used during transaction parsing and address validation flows, so malformed inputs from untrusted transactions could previously reach this code.
Changed components
src/crypto.ccrypto_get_uncompressed_pubkeyLedger Bitcoin app public-key decompressionInspect captured patch +18 / −7
### src/crypto.c
@@ -195,21 +195,32 @@ int crypto_get_uncompressed_pubkey(const uint8_t compressed_key[static 33],
return -1;
}
+ // the x-coordinate must be an element of the base field
+ if (memcmp(compressed_key + 1, secp256k1_p, 32) >= 0) {
+ return -1;
+ }
+
uint8_t *x = &out[1], *y = &out[1 + 32];
memmove(x, compressed_key + 1, 32); // copy x
- // we use y for intermediate results, in order to save memory
+ uint8_t c[32]; // c is the expected value of y^2, that is, x^3 + 7 (mod p)
uint8_t e = 3;
- if (CX_OK != cx_math_powm_no_throw(y, x, &e, 1, secp256k1_p, 32))
- return -1; // tmp = x^3 (mod p)
+ if (CX_OK != cx_math_powm_no_throw(c, x, &e, 1, secp256k1_p, 32)) return -1; // c = x^3 (mod p)
uint8_t scalar[32] = {0};
scalar[31] = 7;
- if (CX_OK != cx_math_addm_no_throw(y, y, scalar, secp256k1_p, 32))
- return -1; // tmp = x^3 + 7 (mod p)
- if (CX_OK != cx_math_powm_no_throw(y, y, secp256k1_sqr_exponent, 32, secp256k1_p, 32))
- return -1; // tmp = sqrt(x^3 + 7) (mod p)
+ if (CX_OK != cx_math_addm_no_throw(c, c, scalar, secp256k1_p, 32))
+ return -1; // c = x^3 + 7 (mod p)
+ if (CX_OK != cx_math_powm_no_throw(y, c, secp256k1_sqr_exponent, 32, secp256k1_p, 32))
+ return -1; // y = sqrt(x^3 + 7) (mod p), if it exists
+
+ // Fail unless y * y % p == x^3 + 7 (guaranteed unless c is not a quadratic residue)
+ uint8_t y_2[32];
+ if (CX_OK != cx_math_multm_no_throw(y_2, y, y, secp256k1_p, 32)) return -1; // y^2 (mod p)
+ if (memcmp(y_2, c, 32) != 0) {
+ return -1; // the point is not on the curve
+ }
// if the prefix and y don't have the same parity, take the opposite root (mod p)
if (((prefix ^ y[31]) & 1) != 0) {Why this scored 61/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.