Skip to content

Commit 98cd21e

Browse files
committed
fix(sol): reject num_signers != 1 proposals before paymaster signs (signer-borrow guard)
1 parent e738d63 commit 98cd21e

2 files changed

Lines changed: 59 additions & 2 deletions

File tree

src/services/solPaymasterService.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -328,10 +328,29 @@ export function validateReimbursement(
328328
}
329329

330330
// Borsh layout after the 8-byte discriminator: u8 vault_index, then
331-
// TransactionMessage = 3×u8 header + Vec<Pubkey> + Vec<CompiledInstruction>
331+
// TransactionMessage = 3×u8 header (num_signers, num_writable_signers,
332+
// num_writable_non_signers) + Vec<Pubkey> + Vec<CompiledInstruction>
332333
// + Vec<AddressTableLookup>.
333334
const data = createIx.data;
334-
let off = 8 + 1 + 3;
335+
let off = 8 + 1; // discriminator + vault_index
336+
337+
// SECURITY (signer-borrow prevention): num_signers decides which accounts get
338+
// is_signer=true when execute_transaction CPIs them. The vault PDA (index 0)
339+
// is the only account the program signs for (its seeds); any additional signer
340+
// slot borrows the top-level signature of the outer tx's fee payer — this
341+
// paymaster — letting the inner CPI move funds FROM the paymaster
342+
// (SystemProgram::transfer) or hijack paymaster-controlled accounts
343+
// (nonceAuthorize / SPL setAuthority) even though vault custody is untouched.
344+
// Every legitimate SSP proposal sets num_signers === 1 (vault only), so reject
345+
// anything else. Mirrors the on-chain require!(num_signers == 1) in
346+
// create_transaction; defense-in-depth for this public broadcast path.
347+
const numSigners = data.readUInt8(off);
348+
if (numSigners !== 1) {
349+
throw new Error(
350+
`Proposal num_signers must be 1 (only the vault PDA may sign); got ${numSigners} — refusing to sponsor a signer-borrow proposal`,
351+
);
352+
}
353+
off += 3; // skip the 3-byte header
335354

336355
const accountKeysLen = data.readUInt32LE(off);
337356
off += 4;

tests/unit/solPaymasterValidateReimbursement.spec.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,4 +279,42 @@ describe('Solana Paymaster Service — validateReimbursement', function () {
279279
/must reimburse paymaster/,
280280
);
281281
});
282+
283+
it('rejects a signer-borrow proposal (num_signers > 1) that would drain the paymaster', function () {
284+
// Pre-mainnet audit regression: at execute time is_signer is taken from
285+
// the proposal header, and the paymaster is the outer-tx fee payer (a
286+
// top-level signer). A proposal marking the paymaster as a second signer
287+
// (index 1 < num_signers = 2) could CPI SystemProgram::transfer FROM the
288+
// paymaster, borrowing its signature — draining it. The reimbursement to
289+
// the paymaster is included so the floor check would pass; the num_signers
290+
// guard must reject it first, before any funds move.
291+
const attacker = Keypair.generate().publicKey;
292+
const message: TransactionMessage = {
293+
numSigners: 2, // illegal: only the vault (index 0) may sign
294+
numWritableSigners: 2,
295+
numWritableNonSigners: 2,
296+
accountKeys: [vault, paymaster, attacker, SystemProgram.programId],
297+
instructions: [
298+
// reimbursement to the paymaster (satisfies the floor if reached)
299+
{
300+
programIdIndex: 3,
301+
accountIndexes: [0, 1],
302+
data: encodeSystemTransferData(MIN_LAMPORTS),
303+
},
304+
// the drain: transfer FROM paymaster (index 1) TO attacker (index 2)
305+
{
306+
programIdIndex: 3,
307+
accountIndexes: [1, 2],
308+
data: encodeSystemTransferData(9_000_000_000),
309+
},
310+
],
311+
};
312+
const tx = makeOuterTx({
313+
paymaster,
314+
ixs: [buildCreateTransactionIx(message)],
315+
});
316+
expect(() => validateReimbursement(tx, paymaster, MIN_LAMPORTS)).to.throw(
317+
/num_signers must be 1/,
318+
);
319+
});
282320
});

0 commit comments

Comments
 (0)