Propagate crypto_tr_lift_x errors in cpoint
What the developer wrote
Authored by Salvatore Ingala
Propagate crypto_tr_lift_x errors in cpoint
What changed, and why it matters
This commit fixes a small but meaningful bug in the Ledger Bitcoin app's code that handles advanced multi-signature (MuSig) operations. A function called crypto_tr_lift_x can fail when given an invalid x-coordinate that does not correspond to any real point on the Bitcoin elliptic curve. Previously, the cpoint function ignored that failure and kept using the resulting output as if it were valid. The patch now checks the return value, prints an error message, and returns an error code instead of continuing with potentially bad data. In a hardware wallet, using invalid curve points could in theory lead to incorrect signature calculations or unexpected behavior, though the practical exploit path is not fully clear from the diff alone.
Treat this as a security-relevant correctness fix. Review whether any other callers of crypto_tr_lift_x ignore its return value, and verify that downstream MuSig code handles cpoint() failure safely. Consider whether a security advisory is warranted if untrusted inputs can reach this path.
Security signals we found
Unchecked return value from cryptographic point-lifting function
Potential use of invalid/uninitialized curve point in MuSig signing flow
Missing error propagation in compressed public-key parsing
Hardware-wallet cryptographic code path touched
Evidence from the diff
The change modifies cpoint() in src/musig/musig.c to propagate the return value of crypto_tr_lift_x(). Previously cpoint() called crypto_tr_lift_x(&x[1], out->raw) without checking its result. crypto_tr_lift_x returns 0 on success and -1 when the supplied 32-byte x is not the x-coordinate of a point on secp256k1. The patch adds an explicit check: if crypto_tr_lift_x returns negative, cpoint returns -1 after logging ‘Invalid compressed point: not on curve’. The header comment in src/crypto.h is updated to document the new contract. This is a defensive correctness fix in the MuSig2 compressed-point parsing path.
Changed components
src/musig/musig.c:cpoint()src/crypto.h:crypto_tr_lift_x() contractLedger Bitcoin app MuSig2 implementationInspect captured patch +7 / −1
### src/crypto.h
@@ -381,6 +381,9 @@ void crypto_tr_tagged_hash_init(cx_sha256_t *hash_context, const uint8_t *tag, u
* Pointer to a 32-byte array.
* @param[out] out
* Pointer to an array that will received the output as an uncompressed 65-bytes pubkey.
+ *
+ * @return 0 on success, -1 if x is not the x-coordinate of a point on the curve. On failure, the
+ * content of `out` is unspecified and must not be used.
*/
int crypto_tr_lift_x(const uint8_t x[static 32], uint8_t out[static 65]);
### src/musig/musig.c
@@ -90,7 +90,10 @@ static bool has_even_y(const point_t *P) {
}
static int cpoint(const uint8_t x[33], point_t *out) {
- crypto_tr_lift_x(&x[1], out->raw);
+ if (0 > crypto_tr_lift_x(&x[1], out->raw)) {
+ PRINTF("Invalid compressed point: not on curve\n");
+ return -1;
+ }
if (is_point_infinite(out)) {
PRINTF("Invalid compressed point\n");
return -1;Why this scored 59/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.