From 17ccdf4133b84c7c41ec21abd574906ae47dd455 Mon Sep 17 00:00:00 2001 From: N V Rakesh Reddy Date: Sat, 18 Jul 2026 20:56:51 +0530 Subject: [PATCH] fix(sdk-coin-avaxp): prevent credential wipe, detect incomplete signing, block invalid broadcast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - hasCredentials: use != null instead of .length > 0 so credentials=[] still blocks buildAvaxTransaction() from regenerating credentials on a partially-signed tx (CECHO-1697 root cause hypothesis) - sign(): guard directly on credentials array (not hasCredentials) since hasCredentials now returns true for [] - signature getter: intersection across all credentials — a signer's ECDSA counted only if present in every credential; RFC6979 determinism means same key → same bytes per input, so missing sig in any credential is detectable before broadcast - isAddrPlaceholder(): distinguish 90-zero-prefix + non-zero address bytes from a fully-empty slot - toBroadcastFormat(): throw when credentials=[] (always a bug state — unsigned txs have credentials=null, not []); throw when real ECDSA coexists with addr placeholder (exact shape from the guard bypass that sent a corrupt tx on Jul 16) References: CECHO-1697 Co-authored-by: Cursor --- .../src/lib/deprecatedTransaction.ts | 71 +++++++- .../test/unit/lib/exportP2CTxBuilder.ts | 162 ++++++++++++++++++ 2 files changed, 228 insertions(+), 5 deletions(-) diff --git a/modules/sdk-coin-avaxp/src/lib/deprecatedTransaction.ts b/modules/sdk-coin-avaxp/src/lib/deprecatedTransaction.ts index d779ac10b1..73b59cfeec 100644 --- a/modules/sdk-coin-avaxp/src/lib/deprecatedTransaction.ts +++ b/modules/sdk-coin-avaxp/src/lib/deprecatedTransaction.ts @@ -37,6 +37,16 @@ function isEmptySignature(s: string): boolean { return !!s && s.startsWith(''.padStart(90, '0')); } +// An address placeholder is 90 zero hex chars (45-byte prefix) followed by a 20-byte address. +// A purely empty slot is all zeros. Non-zero bytes after position 90 distinguish the two. +// This matters for toBroadcastFormat(): a real ECDSA alongside an addr placeholder is the +// exact shape produced by the hasCredentials guard bypass (CECHO-1697) and must be blocked. +function isAddrPlaceholder(s: string): boolean { + if (!isEmptySignature(s)) return false; + const suffix = s.substring(90); + return suffix.length > 0 && suffix !== ''.padStart(suffix.length, '0'); +} + /** * Signatures are prestore as empty buffer for hsm and address of signar for first signature. * When sign is required, this method return the function that identify a signature to be replaced. @@ -94,11 +104,31 @@ export class DeprecatedTransaction extends BaseTransaction { } get signature(): string[] { - if (this.credentials.length === 0) { + if (!this.credentials || this.credentials.length === 0) { return []; } - const obj: any = this.credentials[0].serialize(); - return obj.sigArray.map((s) => s.bytes).filter((s) => !isEmptySignature(s)); + // Use the intersection of non-empty ECDSAs across all credentials. A signer's + // ECDSA is counted only if it appears in every credential (every input). Since + // RFC6979 is deterministic, the same key produces identical bytes for the same + // tx across all credentials. The intersection catches corrupt states where one + // credential is missing a signature that another credential already has. + let intersection: Set | null = null; + for (const c of this.credentials) { + const cs: any = c.serialize(); + const credSigs = new Set( + cs.sigArray.map((s: any) => s.bytes as string).filter((b: string) => !isEmptySignature(b)) + ); + if (intersection === null) { + intersection = credSigs; + } else { + for (const sig of intersection) { + if (!credSigs.has(sig)) { + intersection.delete(sig); + } + } + } + } + return intersection ? [...intersection] : []; } get credentials(): Credential[] { @@ -106,7 +136,10 @@ export class DeprecatedTransaction extends BaseTransaction { } get hasCredentials(): boolean { - return this.credentials !== undefined && this.credentials.length > 0; + // Guard against credential regeneration whenever credentials have been set from a parsed tx. + // Must use != null (not length check) so credentials=[] still blocks buildAvaxTransaction() + // from wiping a partially-signed state. + return this.credentials != null; } /** @inheritdoc */ @@ -133,7 +166,7 @@ export class DeprecatedTransaction extends BaseTransaction { if (!this.avaxPTransaction) { throw new InvalidTransactionError('empty transaction to sign'); } - if (!this.hasCredentials) { + if (!this.credentials || this.credentials.length === 0) { throw new InvalidTransactionError('empty credentials to sign'); } const signature = this.createSignature(prv); @@ -163,6 +196,34 @@ export class DeprecatedTransaction extends BaseTransaction { if (!this.avaxPTransaction) { throw new InvalidTransactionError('Empty transaction data'); } + // Block the exact shape produced by the hasCredentials guard bypass (CECHO-1697): + // a real ECDSA alongside an address placeholder (r=0) in the same credential set. + // Normal states are safe: unsigned txs have credentials=null (not yet set); half-signed + // txs have empty-zero slots (not addr placeholders) in unfilled positions. + // credentials=[] is always a bug: hasCredentials returns true for [] but the array is + // empty, so no signer has touched the tx — serializing it would produce a zero-credential tx. + if (this.credentials != null && this.credentials.length === 0) { + throw new InvalidTransactionError('transaction has no credentials — cannot broadcast'); + } + if (this.credentials && this.credentials.length > 0) { + let hasRealSig = false; + let hasAddrPlaceholder = false; + for (const c of this.credentials) { + const cs: any = c.serialize(); + for (const s of cs.sigArray) { + if (!isEmptySignature(s.bytes)) { + hasRealSig = true; + } else if (isAddrPlaceholder(s.bytes)) { + hasAddrPlaceholder = true; + } + } + } + if (hasRealSig && hasAddrPlaceholder) { + throw new InvalidTransactionError( + 'transaction has a real ECDSA alongside an address placeholder (r=0): incomplete signing detected, refusing broadcast' + ); + } + } return this._avaxTransaction.toStringHex(); } diff --git a/modules/sdk-coin-avaxp/test/unit/lib/exportP2CTxBuilder.ts b/modules/sdk-coin-avaxp/test/unit/lib/exportP2CTxBuilder.ts index 537b895b93..72e0c0087e 100644 --- a/modules/sdk-coin-avaxp/test/unit/lib/exportP2CTxBuilder.ts +++ b/modules/sdk-coin-avaxp/test/unit/lib/exportP2CTxBuilder.ts @@ -151,6 +151,168 @@ describe('AvaxP Export P2C Tx Builder', () => { }, }); + describe('Credential guard bypass regression (CECHO-1697)', () => { + // Regression for production incident 2026-07-16: + // tx 8xbiLpsKDKkJrN3YUqcZpFcxApLCwmQkpKUuvmHU1GL9RmAyu was broadcast + // with sig[1] still an address placeholder (r=0) because hasCredentials([]) + // returned false, causing buildAvaxTransaction() to wipe Signer 1's ECDSA. + const data = testData.EXPORT_P_2_C; + + it('hasCredentials returns true for credentials=[] preventing credential wipe', async () => { + // Signer 1 half-signs + const signer1Builder = factory.from(data.unsignedTxHex); + signer1Builder.sign({ key: data.privKey.prv1 }); + const halfSignedTx = await signer1Builder.build(); + const halfSignedHex = halfSignedTx.toBroadcastFormat(); + + // Simulate the bug trigger: credentials=[] on the parsed tx + const signer2Builder = factory.from(halfSignedHex) as any; + const internalTx = signer2Builder.transaction; + + // Force credentials to [] as happens in production deserialization edge case + (internalTx._avaxTransaction as any).credentials = []; + + // Before fix: hasCredentials was false → buildAvaxTransaction() would regenerate + // After fix: hasCredentials is true → guard fires → credentials preserved + internalTx.hasCredentials.should.be.true( + 'hasCredentials must return true for credentials=[] to block credential regeneration' + ); + }); + + it('credential regeneration is blocked when credentials=[] preventing Signer 1 ECDSA wipe', async () => { + // Signer 1 half-signs + const signer1Builder = factory.from(data.unsignedTxHex); + signer1Builder.sign({ key: data.privKey.prv1 }); + const halfSignedTx = await signer1Builder.build(); + const halfSignedHex = halfSignedTx.toBroadcastFormat(); + + // Simulate bug: force credentials=[] before Signer 2 calls build() + const signer2Builder = factory.from(halfSignedHex) as any; + const internalTx = signer2Builder.transaction; + (internalTx._avaxTransaction as any).credentials = []; + + // build() must NOT regenerate fresh placeholder credentials + const rebuilt = await signer2Builder.build(); + const rebuiltCreds = (rebuilt as any).credentials; + + // After fix: credentials stay as [] (guard fired, no regeneration) + // Before fix: credentials would be regenerated to N×[PLACEHOLDER, PLACEHOLDER] + rebuiltCreds.length.should.equal( + 0, + 'build() must not regenerate credentials when credentials=[] — guard must fire' + ); + }); + + it('signature getter exposes incomplete signing across all credentials', async () => { + // Signer 1 half-signs only + const signer1Builder = factory.from(data.unsignedTxHex); + signer1Builder.sign({ key: data.privKey.prv1 }); + const halfSignedTx = await signer1Builder.build(); + + // After Signer 1: signature getter must show exactly 1 real ECDSA + halfSignedTx.signature.length.should.equal( + 1, + 'should expose exactly 1 real ECDSA after first signer — Signer 2 slot is still empty' + ); + + // Simulate the corrupt state the PR targets: fully sign the tx, then corrupt + // one credential so that credentials[0] has both ECDSAs but credentials[1] + // only has one. A union would still return 2 (masking the corruption). + // The intersection must return 1 because the second signer's ECDSA is absent + // from credentials[1]. + const fullBuilder = factory.from(data.halfsigntxHex); + fullBuilder.sign({ key: data.privKey.prv2 }); + const fullTx = await fullBuilder.build(); + fullTx.signature.length.should.equal(2, 'sanity: fully signed tx should have 2 signatures'); + + // 90 hex chars of leading zeros is the empty-signature prefix used by isEmptySignature(). + const EMPTY_SIG_ZERO_PREFIX = ''.padStart(90, '0'); + + // Corrupt credentials[1]: zero out Signer 2's slot so the intersection drops to 1. + // Use assertions instead of guards so the test fails loudly if the tx shape changes. + const creds = (fullTx as any).credentials; + assert.ok(creds && creds.length >= 2, 'fully signed tx must have at least 2 credentials'); + const cred1: any = creds[1]; + const cs1: any = cred1.serialize(); + const targetIdx = cs1.sigArray.findIndex( + (s: any, i: number) => i > 0 && !s.bytes.startsWith(EMPTY_SIG_ZERO_PREFIX) + ); + assert.ok(targetIdx !== -1, 'credentials[1] must have a non-empty slot beyond index 0 (Signer 2 slot)'); + cs1.sigArray[targetIdx].bytes = ''.padStart(cs1.sigArray[targetIdx].bytes.length, '0'); + cred1.deserialize(cs1); + + // With intersection, removing ECDSA2 from credentials[1] must reduce the count to 1. + fullTx.signature.length.should.equal( + 1, + 'intersection must detect that credentials[1] is missing ECDSA2 — incomplete signing visible' + ); + }); + + it('full sign from half-signed hex produces 2 real ECDSAs and correct tx', async () => { + // This is the exact flow that MUST work in production after the fix: + // Signer 2 takes Signer 1's half-signed hex and fills the remaining slot. + const txBuilder = factory.from(data.halfsigntxHex); + txBuilder.sign({ key: data.privKey.prv2 }); + const tx = await txBuilder.build(); + tx.toBroadcastFormat().should.equal(data.fullsigntxHex); + tx.signature.length.should.equal(2, 'both HSM signer slots must be filled with real ECDSAs'); + }); + + it('sign() throws on empty credentials rather than silently producing bad tx', async () => { + // Signer 1 half-signs + const signer1Builder = factory.from(data.unsignedTxHex); + signer1Builder.sign({ key: data.privKey.prv1 }); + const halfTx = await signer1Builder.build(); + + // Simulate bug: credentials wiped to [] before Signer 2 signs + const signer2Builder = factory.from(halfTx.toBroadcastFormat()) as any; + signer2Builder.sign({ key: data.privKey.prv2 }); + (signer2Builder.transaction._avaxTransaction as any).credentials = []; + + // build() calls transaction.sign() which must throw — not silently produce a bad tx + await signer2Builder + .build() + .then(() => assert.fail('Expected sign to throw on empty credentials')) + .catch((e: any) => { + e.message.should.equal('empty credentials to sign'); + }); + }); + + it('toBroadcastFormat() throws when real ECDSA coexists with address placeholder (production failure shape)', async () => { + // Reproduce the exact credential layout from the Jul 16 production failure: + // slot[0] = real ECDSA (Signer 2 filled wrong slot due to MODE 1 mismatch) + // slot[1] = ADDR_PLACEHOLDER (r=0, 90 zero hex chars + HSM1 address) + // This is the shape that AvalancheGo rejected with "failed verifySpend: invalid signature". + const EMPTY_SIG_ZERO_PREFIX = ''.padStart(90, '0'); + const signer1Builder = factory.from(data.unsignedTxHex); + signer1Builder.sign({ key: data.privKey.prv1 }); + const halfTx = await signer1Builder.build(); + const fullBuilder = factory.from(halfTx.toBroadcastFormat()); + fullBuilder.sign({ key: data.privKey.prv2 }); + const fullTx = (await fullBuilder.build()) as any; + + // Corrupt credentials: swap slot[0] (realECDSA1) with an addr placeholder, + // leaving slot[1] = realECDSA2. This mimics the bug output where a real ECDSA + // coexists with an addr placeholder in the same credential. + const creds = fullTx.credentials; + if (creds && creds.length > 0) { + const cred0: any = creds[0]; + const cs: any = cred0.serialize(); + // Build a fake addr placeholder: 90 zeros + 40 hex chars of a mock address + const fakeAddrPlaceholder = EMPTY_SIG_ZERO_PREFIX + 'df32717bd7b7a2d50a715202795940250c7ba9e4'; + cs.sigArray[0].bytes = fakeAddrPlaceholder; + cred0.deserialize(cs); + } + + assert.throws( + () => fullTx.toBroadcastFormat(), + (e: any) => + e.message === + 'transaction has a real ECDSA alongside an address placeholder (r=0): incomplete signing detected, refusing broadcast' + ); + }); + }); + describe('Key cannot sign the transaction ', () => { const data = testData.EXPORT_P_2_C; it('Should full sign a export tx from unsigned raw tx', () => {