node icon indicating copy to clipboard operation
node copied to clipboard

Incorrect Format for transaction building will lead to revert execute.

Open lumtis opened this issue 6 months ago • 0 comments

Sherlock low severity finding

Affected Code

https://github.com/zeta-chain/node/blob/a32ffd88ea9957c8f3424466bb358b4197d41889/zetaclient/chains/solana/signer/execute.go#L88-L89

When creating Execute Function in Solana, The Account structure us making destination_program with no mut flag (non-writable), and destination_program_pda with mut flag (writable)

pub struct Execute<'info> {
    ...
    pub signer: Signer<'info>,
    ...
    pub pda: Account<'info, Pda>,

    /// The destination program.
    /// CHECK: This is arbitrary program.
>>  pub destination_program: AccountInfo<'info>,
    ...
    #[account(
>>      mut,
        ....
    )]
>>  pub destination_program_pda: UncheckedAccount<'info>,
}

But when constructing the tx by Solana Nodes, they are doing the reverse, by making the destination_program as writable and destination_program_pda as non-writable

	predefinedAccounts := []*solana.AccountMeta{
		solana.Meta(signer.relayerKey.PublicKey()).WRITE().SIGNER(),
		solana.Meta(signer.pda).WRITE(),
		solana.Meta(msg.To()).WRITE(), <<---------
		solana.Meta(destinationProgramPda), <<---------
	}

This will result in All transactions gets reverted, as the pda account is the account designed to be changed in the execution of on_call and this can be seen cealy in example folders

    pub fn on_call( ... ) -> Result<()> {
>>      let pda = &mut ctx.accounts.pda;

        // Store the sender's public key
>>      pda.last_sender = sender;

        // Convert data to a string and store it
        let message = String::from_utf8(data).map_err(|_| ErrorCode::InvalidDataFormat)?;
>>      pda.last_message = message;

        // Transfer some portion of lamports transferred from gateway to another account
>>      pda.sub_lamports(amount / 2)?;
    }

Destination_PDA is the trusted account, that Cross Chain Apps depend on changing it. But as all Relayer txs are signed either in execute function, do not make it as writable, most of execute signed txs by the Node Relayers will get reverted as it will try to write on unwritable account leading to revert in execution

Recommendation

in execute.go remove WRITE from msg.To, which is the destination_program, and it it to destinationProgramPda

lumtis avatar Jun 20 '25 11:06 lumtis