Skip to content

Commit cebf2a2

Browse files
committed
fix(sdk-coin-avaxp): prevent credential wipe, detect incomplete signing, block invalid broadcast
- hasCredentials: use != null instead of .length > 0 so credentials=[] still blocks buildAvaxTransaction() from regenerating credentials on a partially-signed tx (CECHO-1697 root cause) - sign(): guard directly on credentials array, not hasCredentials - signature getter: use intersection of ECDSAs across all credentials (RFC6979 determinism means same key → same bytes per input; missing a sig in any credential is now detectable) - isAddrPlaceholder(): distinguish 90-zero-prefix + non-zero address from a fully-empty slot - toBroadcastFormat(): throw if real ECDSA coexists with an address placeholder — exact shape produced by the guard bypass that sent a corrupt tx on Jul 16 References: CECHO-1697
1 parent 4e91dde commit cebf2a2

2 files changed

Lines changed: 223 additions & 5 deletions

File tree

modules/sdk-coin-avaxp/src/lib/deprecatedTransaction.ts

Lines changed: 61 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,16 @@ function isEmptySignature(s: string): boolean {
3737
return !!s && s.startsWith(''.padStart(90, '0'));
3838
}
3939

40+
// An address placeholder is 90 zero hex chars (45-byte prefix) followed by a 20-byte address.
41+
// A purely empty slot is all zeros. Non-zero bytes after position 90 distinguish the two.
42+
// This matters for toBroadcastFormat(): a real ECDSA alongside an addr placeholder is the
43+
// exact shape produced by the hasCredentials guard bypass (CECHO-1697) and must be blocked.
44+
function isAddrPlaceholder(s: string): boolean {
45+
if (!isEmptySignature(s)) return false;
46+
const suffix = s.substring(90);
47+
return suffix.length > 0 && suffix !== ''.padStart(suffix.length, '0');
48+
}
49+
4050
/**
4151
* Signatures are prestore as empty buffer for hsm and address of signar for first signature.
4252
* When sign is required, this method return the function that identify a signature to be replaced.
@@ -94,19 +104,42 @@ export class DeprecatedTransaction extends BaseTransaction {
94104
}
95105

96106
get signature(): string[] {
97-
if (this.credentials.length === 0) {
107+
if (!this.credentials || this.credentials.length === 0) {
98108
return [];
99109
}
100-
const obj: any = this.credentials[0].serialize();
101-
return obj.sigArray.map((s) => s.bytes).filter((s) => !isEmptySignature(s));
110+
// Use the intersection of non-empty ECDSAs across all credentials. A signer's
111+
// ECDSA is counted only if it appears in every credential (every input). Since
112+
// RFC6979 is deterministic, the same key produces identical bytes for the same
113+
// tx across all credentials. The intersection catches corrupt states where one
114+
// credential is missing a signature that another credential already has.
115+
let intersection: Set<string> | null = null;
116+
for (const c of this.credentials) {
117+
const cs: any = c.serialize();
118+
const credSigs = new Set<string>(
119+
cs.sigArray.map((s: any) => s.bytes as string).filter((b: string) => !isEmptySignature(b))
120+
);
121+
if (intersection === null) {
122+
intersection = credSigs;
123+
} else {
124+
for (const sig of intersection) {
125+
if (!credSigs.has(sig)) {
126+
intersection.delete(sig);
127+
}
128+
}
129+
}
130+
}
131+
return intersection ? [...intersection] : [];
102132
}
103133

104134
get credentials(): Credential[] {
105135
return (this._avaxTransaction as any)?.credentials;
106136
}
107137

108138
get hasCredentials(): boolean {
109-
return this.credentials !== undefined && this.credentials.length > 0;
139+
// Guard against credential regeneration whenever credentials have been set from a parsed tx.
140+
// Must use != null (not length check) so credentials=[] still blocks buildAvaxTransaction()
141+
// from wiping a partially-signed state.
142+
return this.credentials != null;
110143
}
111144

112145
/** @inheritdoc */
@@ -133,7 +166,7 @@ export class DeprecatedTransaction extends BaseTransaction {
133166
if (!this.avaxPTransaction) {
134167
throw new InvalidTransactionError('empty transaction to sign');
135168
}
136-
if (!this.hasCredentials) {
169+
if (!this.credentials || this.credentials.length === 0) {
137170
throw new InvalidTransactionError('empty credentials to sign');
138171
}
139172
const signature = this.createSignature(prv);
@@ -163,6 +196,29 @@ export class DeprecatedTransaction extends BaseTransaction {
163196
if (!this.avaxPTransaction) {
164197
throw new InvalidTransactionError('Empty transaction data');
165198
}
199+
// Block the exact shape produced by the hasCredentials guard bypass (CECHO-1697):
200+
// a real ECDSA alongside an address placeholder (r=0) in the same credential set.
201+
// Normal states are safe: unsigned txs have no real ECDSAs yet; half-signed txs have
202+
// empty-zero slots (not addr placeholders) in unfilled positions.
203+
if (this.credentials && this.credentials.length > 0) {
204+
let hasRealSig = false;
205+
let hasAddrPlaceholder = false;
206+
for (const c of this.credentials) {
207+
const cs: any = c.serialize();
208+
for (const s of cs.sigArray) {
209+
if (!isEmptySignature(s.bytes)) {
210+
hasRealSig = true;
211+
} else if (isAddrPlaceholder(s.bytes)) {
212+
hasAddrPlaceholder = true;
213+
}
214+
}
215+
}
216+
if (hasRealSig && hasAddrPlaceholder) {
217+
throw new InvalidTransactionError(
218+
'transaction has a real ECDSA alongside an address placeholder (r=0): incomplete signing detected, refusing broadcast'
219+
);
220+
}
221+
}
166222
return this._avaxTransaction.toStringHex();
167223
}
168224

modules/sdk-coin-avaxp/test/unit/lib/exportP2CTxBuilder.ts

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,168 @@ describe('AvaxP Export P2C Tx Builder', () => {
151151
},
152152
});
153153

154+
describe('Credential guard bypass regression (CECHO-1697)', () => {
155+
// Regression for production incident 2026-07-16:
156+
// tx 8xbiLpsKDKkJrN3YUqcZpFcxApLCwmQkpKUuvmHU1GL9RmAyu was broadcast
157+
// with sig[1] still an address placeholder (r=0) because hasCredentials([])
158+
// returned false, causing buildAvaxTransaction() to wipe Signer 1's ECDSA.
159+
const data = testData.EXPORT_P_2_C;
160+
161+
it('hasCredentials returns true for credentials=[] preventing credential wipe', async () => {
162+
// Signer 1 half-signs
163+
const signer1Builder = factory.from(data.unsignedTxHex);
164+
signer1Builder.sign({ key: data.privKey.prv1 });
165+
const halfSignedTx = await signer1Builder.build();
166+
const halfSignedHex = halfSignedTx.toBroadcastFormat();
167+
168+
// Simulate the bug trigger: credentials=[] on the parsed tx
169+
const signer2Builder = factory.from(halfSignedHex) as any;
170+
const internalTx = signer2Builder.transaction;
171+
172+
// Force credentials to [] as happens in production deserialization edge case
173+
(internalTx._avaxTransaction as any).credentials = [];
174+
175+
// Before fix: hasCredentials was false → buildAvaxTransaction() would regenerate
176+
// After fix: hasCredentials is true → guard fires → credentials preserved
177+
internalTx.hasCredentials.should.be.true(
178+
'hasCredentials must return true for credentials=[] to block credential regeneration'
179+
);
180+
});
181+
182+
it('credential regeneration is blocked when credentials=[] preventing Signer 1 ECDSA wipe', async () => {
183+
// Signer 1 half-signs
184+
const signer1Builder = factory.from(data.unsignedTxHex);
185+
signer1Builder.sign({ key: data.privKey.prv1 });
186+
const halfSignedTx = await signer1Builder.build();
187+
const halfSignedHex = halfSignedTx.toBroadcastFormat();
188+
189+
// Simulate bug: force credentials=[] before Signer 2 calls build()
190+
const signer2Builder = factory.from(halfSignedHex) as any;
191+
const internalTx = signer2Builder.transaction;
192+
(internalTx._avaxTransaction as any).credentials = [];
193+
194+
// build() must NOT regenerate fresh placeholder credentials
195+
const rebuilt = await signer2Builder.build();
196+
const rebuiltCreds = (rebuilt as any).credentials;
197+
198+
// After fix: credentials stay as [] (guard fired, no regeneration)
199+
// Before fix: credentials would be regenerated to N×[PLACEHOLDER, PLACEHOLDER]
200+
rebuiltCreds.length.should.equal(
201+
0,
202+
'build() must not regenerate credentials when credentials=[] — guard must fire'
203+
);
204+
});
205+
206+
it('signature getter exposes incomplete signing across all credentials', async () => {
207+
// Signer 1 half-signs only
208+
const signer1Builder = factory.from(data.unsignedTxHex);
209+
signer1Builder.sign({ key: data.privKey.prv1 });
210+
const halfSignedTx = await signer1Builder.build();
211+
212+
// After Signer 1: signature getter must show exactly 1 real ECDSA
213+
halfSignedTx.signature.length.should.equal(
214+
1,
215+
'should expose exactly 1 real ECDSA after first signer — Signer 2 slot is still empty'
216+
);
217+
218+
// Simulate the corrupt state the PR targets: fully sign the tx, then corrupt
219+
// one credential so that credentials[0] has both ECDSAs but credentials[1]
220+
// only has one. A union would still return 2 (masking the corruption).
221+
// The intersection must return 1 because the second signer's ECDSA is absent
222+
// from credentials[1].
223+
const fullBuilder = factory.from(data.halfsigntxHex);
224+
fullBuilder.sign({ key: data.privKey.prv2 });
225+
const fullTx = await fullBuilder.build();
226+
fullTx.signature.length.should.equal(2, 'sanity: fully signed tx should have 2 signatures');
227+
228+
// 90 hex chars of leading zeros is the empty-signature prefix used by isEmptySignature().
229+
const EMPTY_SIG_ZERO_PREFIX = ''.padStart(90, '0');
230+
231+
// Corrupt credentials[1]: zero out Signer 2's slot so the intersection drops to 1.
232+
// Use assertions instead of guards so the test fails loudly if the tx shape changes.
233+
const creds = (fullTx as any).credentials;
234+
assert.ok(creds && creds.length >= 2, 'fully signed tx must have at least 2 credentials');
235+
const cred1: any = creds[1];
236+
const cs1: any = cred1.serialize();
237+
const targetIdx = cs1.sigArray.findIndex(
238+
(s: any, i: number) => i > 0 && !s.bytes.startsWith(EMPTY_SIG_ZERO_PREFIX)
239+
);
240+
assert.ok(targetIdx !== -1, 'credentials[1] must have a non-empty slot beyond index 0 (Signer 2 slot)');
241+
cs1.sigArray[targetIdx].bytes = ''.padStart(cs1.sigArray[targetIdx].bytes.length, '0');
242+
cred1.deserialize(cs1);
243+
244+
// With intersection, removing ECDSA2 from credentials[1] must reduce the count to 1.
245+
fullTx.signature.length.should.equal(
246+
1,
247+
'intersection must detect that credentials[1] is missing ECDSA2 — incomplete signing visible'
248+
);
249+
});
250+
251+
it('full sign from half-signed hex produces 2 real ECDSAs and correct tx', async () => {
252+
// This is the exact flow that MUST work in production after the fix:
253+
// Signer 2 takes Signer 1's half-signed hex and fills the remaining slot.
254+
const txBuilder = factory.from(data.halfsigntxHex);
255+
txBuilder.sign({ key: data.privKey.prv2 });
256+
const tx = await txBuilder.build();
257+
tx.toBroadcastFormat().should.equal(data.fullsigntxHex);
258+
tx.signature.length.should.equal(2, 'both HSM signer slots must be filled with real ECDSAs');
259+
});
260+
261+
it('sign() throws on empty credentials rather than silently producing bad tx', async () => {
262+
// Signer 1 half-signs
263+
const signer1Builder = factory.from(data.unsignedTxHex);
264+
signer1Builder.sign({ key: data.privKey.prv1 });
265+
const halfTx = await signer1Builder.build();
266+
267+
// Simulate bug: credentials wiped to [] before Signer 2 signs
268+
const signer2Builder = factory.from(halfTx.toBroadcastFormat()) as any;
269+
signer2Builder.sign({ key: data.privKey.prv2 });
270+
(signer2Builder.transaction._avaxTransaction as any).credentials = [];
271+
272+
// build() calls transaction.sign() which must throw — not silently produce a bad tx
273+
await signer2Builder
274+
.build()
275+
.then(() => assert.fail('Expected sign to throw on empty credentials'))
276+
.catch((e: any) => {
277+
e.message.should.equal('empty credentials to sign');
278+
});
279+
});
280+
281+
it('toBroadcastFormat() throws when real ECDSA coexists with address placeholder (production failure shape)', async () => {
282+
// Reproduce the exact credential layout from the Jul 16 production failure:
283+
// slot[0] = real ECDSA (Signer 2 filled wrong slot due to MODE 1 mismatch)
284+
// slot[1] = ADDR_PLACEHOLDER (r=0, 90 zero hex chars + HSM1 address)
285+
// This is the shape that AvalancheGo rejected with "failed verifySpend: invalid signature".
286+
const EMPTY_SIG_ZERO_PREFIX = ''.padStart(90, '0');
287+
const signer1Builder = factory.from(data.unsignedTxHex);
288+
signer1Builder.sign({ key: data.privKey.prv1 });
289+
const halfTx = await signer1Builder.build();
290+
const fullBuilder = factory.from(halfTx.toBroadcastFormat());
291+
fullBuilder.sign({ key: data.privKey.prv2 });
292+
const fullTx = await fullBuilder.build() as any;
293+
294+
// Corrupt credentials: swap slot[0] (realECDSA1) with an addr placeholder,
295+
// leaving slot[1] = realECDSA2. This mimics the bug output where a real ECDSA
296+
// coexists with an addr placeholder in the same credential.
297+
const creds = fullTx.credentials;
298+
if (creds && creds.length > 0) {
299+
const cred0: any = creds[0];
300+
const cs: any = cred0.serialize();
301+
// Build a fake addr placeholder: 90 zeros + 40 hex chars of a mock address
302+
const fakeAddrPlaceholder = EMPTY_SIG_ZERO_PREFIX + 'df32717bd7b7a2d50a715202795940250c7ba9e4';
303+
cs.sigArray[0].bytes = fakeAddrPlaceholder;
304+
cred0.deserialize(cs);
305+
}
306+
307+
assert.throws(
308+
() => fullTx.toBroadcastFormat(),
309+
(e: any) =>
310+
e.message ===
311+
'transaction has a real ECDSA alongside an address placeholder (r=0): incomplete signing detected, refusing broadcast'
312+
);
313+
});
314+
});
315+
154316
describe('Key cannot sign the transaction ', () => {
155317
const data = testData.EXPORT_P_2_C;
156318
it('Should full sign a export tx from unsigned raw tx', () => {

0 commit comments

Comments
 (0)