← Watch feed
sign_tx: remove blind proofs from commitment data
Public commit record
68/100 · Adequate
What the developer wrote
Authored by Jon Griffiths
sign_tx: remove blind proofs from commitment data
As with the asset and value commitments, these are not required once the
commitment data has been validated.
Re-order the commitment struct members to further lower the memory size
required to hold commitments.
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This commit reorganizes how a hardware wallet stores temporary blinding proofs while validating a confidential transaction. It moves large proof data out of a small 'commitment' structure into a larger extended structure, and only copies the smaller validated data back to the caller. The stated goal is to reduce memory use, not to fix a security bug. There is no direct evidence in the commit that this prevents an exploitable vulnerability, but memory handling changes in security-critical code always warrant careful review.
Recommended action
Review whether any caller previously relied on commitment_t containing the blind proofs after params_commitment_data() returns, and confirm that the reduced struct size does not introduce use-of-uninitialized-data or information-leak risks. Treat as a hardening/memory-hygiene change rather than an urgent vulnerability patch unless additional evidence emerges.
Security signals we found
01
Memory layout change in cryptographic commitment handling
02
Large proof buffers moved out of a structure that is copied back to callers
03
Explicit proof verification still occurs before returning data
04
No bounds-check changes or new input sanitization visible in diff
05
Commit message frames change as optimization, not security fix
Evidence from the diff
The patch refactors commitment_t and ext_commitment_t in Blockstream Jade’s transaction signing path. Previously commitment_t held asset_blind_proof, value_blind_proof, and value_blind_proof_len; these are now moved into ext_commitment_t. params_commitment_data() parses RPC fields into a local ext_commitment_t, validates abf/vbf against asset_generator/value_commitment, optionally verifies explicit surjection/range proofs in a temporary high-stack task, and finally copies only the base commitment_t (without the proofs) back to the caller. The commit message says this is for memory optimization after validation, analogous to earlier removal of asset/value commitments from commitment data.
Changed components
main/process/process_utils.hmain/process/sign_utils.cTransaction signing / confidential asset proof verification pathInspect captured patch +38 / −37
diff --git a/main/process/process_utils.h b/main/process/process_utils.h
index 48dc711..c03f974 100644
--- a/main/process/process_utils.h
+++ b/main/process/process_utils.h
@@ -16,20 +16,20 @@
// Holds asset/value blinding data
typedef struct {
- uint8_t asset_blind_proof[ASSET_EXPLICIT_SURJECTIONPROOF_LEN];
- uint8_t value_blind_proof[ASSET_EXPLICIT_RANGEPROOF_MAX_LEN];
+ uint64_t value;
uint8_t asset_id[ASSET_TAG_LEN];
uint8_t abf[BLINDING_FACTOR_LEN];
uint8_t vbf[BLINDING_FACTOR_LEN];
uint8_t blinding_key[EC_PUBLIC_KEY_LEN];
- uint64_t value;
- uint8_t value_blind_proof_len;
uint8_t content;
} commitment_t;
-// Holds asset/value blinding data plus the resulting blinded commitments
+// Holds asset/value blinding data plus the resulting blinded commitments/proofs
typedef struct {
commitment_t c;
+ uint8_t value_blind_proof_len;
+ uint8_t asset_blind_proof[ASSET_EXPLICIT_SURJECTIONPROOF_LEN];
+ uint8_t value_blind_proof[ASSET_EXPLICIT_RANGEPROOF_MAX_LEN];
uint8_t asset_generator[ASSET_GENERATOR_LEN];
uint8_t value_commitment[ASSET_COMMITMENT_LEN];
} ext_commitment_t;
diff --git a/main/process/sign_utils.c b/main/process/sign_utils.c
index 82e3ed8..4fc88dc 100644
--- a/main/process/sign_utils.c
+++ b/main/process/sign_utils.c
@@ -293,8 +293,8 @@ static bool verify_explicit_proofs(void* ctx)
reverse(reversed_asset_id, c->asset_id, sizeof(c->asset_id));
// NOTE: Appears to require ~52kb of stack space
- if (wally_explicit_surjectionproof_verify(c->asset_blind_proof, sizeof(c->asset_blind_proof), reversed_asset_id,
- sizeof(reversed_asset_id), ec->asset_generator, sizeof(ec->asset_generator))
+ if (wally_explicit_surjectionproof_verify(ec->asset_blind_proof, sizeof(ec->asset_blind_proof),
+ reversed_asset_id, sizeof(reversed_asset_id), ec->asset_generator, sizeof(ec->asset_generator))
!= WALLY_OK) {
// Failed to verify explicit asset proof
return false;
@@ -303,7 +303,7 @@ static bool verify_explicit_proofs(void* ctx)
if (c->content & COMMITMENTS_VALUE_BLIND_PROOF) {
// NOTE: Appears to require ~40kb of stack space
- if (wally_explicit_rangeproof_verify(c->value_blind_proof, c->value_blind_proof_len, c->value,
+ if (wally_explicit_rangeproof_verify(ec->value_blind_proof, ec->value_blind_proof_len, c->value,
ec->value_commitment, sizeof(ec->value_commitment), ec->asset_generator, sizeof(ec->asset_generator))
!= WALLY_OK) {
// Failed to verify explicit value proof
@@ -325,44 +325,45 @@ bool params_commitment_data(
commitment->content = COMMITMENTS_NONE;
+ ext_commitment_t ec;
+ ec.c.content = COMMITMENTS_NONE;
+
// Need abf or asset_blind_proof
- if (rpc_get_n_bytes("abf", item, sizeof(commitment->abf), commitment->abf)) {
- commitment->content |= COMMITMENTS_ABF;
+ if (rpc_get_n_bytes("abf", item, sizeof(ec.c.abf), ec.c.abf)) {
+ ec.c.content |= COMMITMENTS_ABF;
}
- if (rpc_get_n_bytes(
- "asset_blind_proof", item, sizeof(commitment->asset_blind_proof), commitment->asset_blind_proof)) {
- commitment->content |= COMMITMENTS_ASSET_BLIND_PROOF;
+ if (rpc_get_n_bytes("asset_blind_proof", item, sizeof(ec.asset_blind_proof), ec.asset_blind_proof)) {
+ ec.c.content |= COMMITMENTS_ASSET_BLIND_PROOF;
}
- if (!(commitment->content & (COMMITMENTS_ABF | COMMITMENTS_ASSET_BLIND_PROOF))) {
+ if (!(ec.c.content & (COMMITMENTS_ABF | COMMITMENTS_ASSET_BLIND_PROOF))) {
// No commitment data present
return false;
}
// Need vbf or value_blind_proof
- if (rpc_get_n_bytes("vbf", item, sizeof(commitment->vbf), commitment->vbf)) {
- commitment->content |= COMMITMENTS_VBF;
+ if (rpc_get_n_bytes("vbf", item, sizeof(ec.c.vbf), ec.c.vbf)) {
+ ec.c.content |= COMMITMENTS_VBF;
}
size_t written = 0;
- rpc_get_bytes(
- "value_blind_proof", sizeof(commitment->value_blind_proof), item, commitment->value_blind_proof, &written);
- if (written && written <= sizeof(commitment->value_blind_proof)) {
- commitment->value_blind_proof_len = (uint8_t)written; // Sufficient
- commitment->content |= COMMITMENTS_VALUE_BLIND_PROOF;
+ rpc_get_bytes("value_blind_proof", sizeof(ec.value_blind_proof), item, ec.value_blind_proof, &written);
+ if (written && written <= sizeof(ec.value_blind_proof)) {
+ ec.value_blind_proof_len = (uint8_t)written; // Sufficient
+ ec.c.content |= COMMITMENTS_VALUE_BLIND_PROOF;
}
- if (!(commitment->content & (COMMITMENTS_VBF | COMMITMENTS_VALUE_BLIND_PROOF))
- || !rpc_get_n_bytes("asset_id", item, sizeof(commitment->asset_id), commitment->asset_id)
- || !rpc_get_uint64_t("value", item, &commitment->value)) {
+ if (!(ec.c.content & (COMMITMENTS_VBF | COMMITMENTS_VALUE_BLIND_PROOF))
+ || !rpc_get_n_bytes("asset_id", item, sizeof(ec.c.asset_id), ec.c.asset_id)
+ || !rpc_get_uint64_t("value", item, &ec.c.value)) {
*errmsg = "Invalid or missing trusted commitment data";
return false;
}
// Blinding key is optional in some scenarios
- if (rpc_get_n_bytes("blinding_key", item, sizeof(commitment->blinding_key), commitment->blinding_key)) {
- commitment->content |= COMMITMENTS_BLINDING_KEY;
+ if (rpc_get_n_bytes("blinding_key", item, sizeof(ec.c.blinding_key), ec.c.blinding_key)) {
+ ec.c.content |= COMMITMENTS_BLINDING_KEY;
}
// For tx output commitments:
@@ -373,7 +374,6 @@ bool params_commitment_data(
// - Actual commitments are mandatory
//
// The above blinding factors/proofs are then verified against the commitments.
- ext_commitment_t ec;
const bool have_asset_generator
= rpc_get_n_bytes("asset_generator", item, sizeof(ec.asset_generator), ec.asset_generator);
const bool have_value_commitment
@@ -412,13 +412,13 @@ bool params_commitment_data(
// 1. Asset generator
// If passed the abf, check the blinded asset commitment can be reconstructed
// (ie. from the given reversed asset_id and abf)
- if (commitment->content & COMMITMENTS_ABF) {
- uint8_t reversed_asset_id[sizeof(commitment->asset_id)];
- reverse(reversed_asset_id, commitment->asset_id, sizeof(commitment->asset_id));
+ if (ec.c.content & COMMITMENTS_ABF) {
+ uint8_t reversed_asset_id[sizeof(ec.c.asset_id)];
+ reverse(reversed_asset_id, ec.c.asset_id, sizeof(ec.c.asset_id));
uint8_t cmp[sizeof(ec.asset_generator)];
- if (wally_asset_generator_from_bytes(reversed_asset_id, sizeof(reversed_asset_id), commitment->abf,
- sizeof(commitment->abf), cmp, sizeof(cmp))
+ if (wally_asset_generator_from_bytes(
+ reversed_asset_id, sizeof(reversed_asset_id), ec.c.abf, sizeof(ec.c.abf), cmp, sizeof(cmp))
!= WALLY_OK
|| sodium_memcmp(ec.asset_generator, cmp, sizeof(cmp)) != 0) {
*errmsg = "Failed to verify trusted commitment data with tx";
@@ -429,10 +429,10 @@ bool params_commitment_data(
// 2. Value commitment
// If passed the vbf, check the blinded value commitment can be reconstructed
// (ie. from the given value, asset_generator and vbf)
- if (commitment->content & COMMITMENTS_VBF) {
+ if (ec.c.content & COMMITMENTS_VBF) {
uint8_t cmp[sizeof(ec.value_commitment)];
- if (wally_asset_value_commitment(commitment->value, commitment->vbf, sizeof(commitment->vbf),
- ec.asset_generator, sizeof(ec.asset_generator), cmp, sizeof(cmp))
+ if (wally_asset_value_commitment(ec.c.value, ec.c.vbf, sizeof(ec.c.vbf), ec.asset_generator,
+ sizeof(ec.asset_generator), cmp, sizeof(cmp))
!= WALLY_OK
|| sodium_memcmp(ec.value_commitment, cmp, sizeof(cmp)) != 0) {
*errmsg = "Failed to verify trusted commitment data with tx";
@@ -442,12 +442,11 @@ bool params_commitment_data(
// Verify any blinded proofs
// NOTE: only a device with SPIRAM has sufficient memory to be able to do this verification.
- if (commitment->content & (COMMITMENTS_ASSET_BLIND_PROOF | COMMITMENTS_VALUE_BLIND_PROOF)) {
+ if (ec.c.content & (COMMITMENTS_ASSET_BLIND_PROOF | COMMITMENTS_VALUE_BLIND_PROOF)) {
#ifdef CONFIG_SPIRAM
// Because the libsecp calls 'secp256k1_surjectionproof_verify()' and 'secp256k1_rangeproof_verify()'
// requires more stack space than is available to the main task, we run that function in a temporary task.
const size_t stack_size = 54 * 1024; // 54kb seems sufficient
- memcpy(&ec.c, commitment, sizeof(*commitment));
if (!run_in_temporary_task(stack_size, verify_explicit_proofs, (void*)&ec)) {
*errmsg = "Failed to verify explicit asset/value commitment proofs";
return false;
@@ -457,6 +456,8 @@ bool params_commitment_data(
return false;
#endif // CONFIG_SPIRAM
}
+ // Copy out the valid commitment data for the caller
+ memcpy(commitment, &ec.c, sizeof(ec.c));
return true;
}
Risk score
Our methodology →Why this scored 25/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.