Dizzy Green Mantis - Inconsistent `AccountDepositSol` struct in `ILedger.sol` and `msd_codec.rs`

[sherlock-admin3]

Dizzy Green Mantis

High

Inconsistent AccountDepositSol struct in ILedger.sol and msd_codec.rs

Summary

The protocol uses LayerZero to do cross network communication. Users can deposit on the Solana vault, and it will be forwarded through LZ, eventually landing Ethereum's SolConnector. However, during message decoding process, there is a discrepancy between sent and received struct, which can cause unexpected value to be parsed or DoS.

Root Cause

In _lzReceive, the deposit message will be decoded and forwarded to ledger contract:

    function _lzReceive(
        Origin calldata _origin,
        bytes32 /*_guid*/,
        bytes calldata _message,
        address /*_executor*/,
        bytes calldata /*_extraData*/
    ) internal virtual override {
        if (orderDelivery) {
            require(_origin.nonce == inboundNonce + 1, "Invalid inbound nonce");
        }
        inboundNonce = _origin.nonce;

        (uint8 msgType, bytes memory payload) = MsgCodec.decodeLzMsg(_message);

        if (msgType == uint8(MsgCodec.MsgType.Deposit)) {
            AccountDepositSol memory accountDepositSol = abi.decode(payload, (AccountDepositSol));
            ledger.accountDepositSol(accountDepositSol);
        } else {
            emit UnkonwnMessageType(msgType);
        }
    }

and here is the decode function:

    function decodeLzMsg(bytes calldata _msg) internal pure returns (uint8 msgType, bytes memory payload) {
        msgType = uint8(bytes1(_msg[:MSG_TYPE_OFFSET]));
        payload = _msg[MSG_TYPE_OFFSET:];
    }

Which takes the first byte of message, checking if it's deposit type, and unmarshal the rest into AccountDepositSol. On the other hand, programs on Solana offers a method deposit, and it does similar job as SolConnector::withdraw:

    pub fn apply(
        ctx: &mut Context<'_, '_, '_, 'info, Deposit<'info>>,
        deposit_params: &DepositParams,
        oapp_params: &OAppSendParams,
    ) -> Result<MessagingReceipt> {
        transfer(
            ctx.accounts.transfer_token_ctx(),
            deposit_params.token_amount,
        )?;

        msg!("User deposited : {}", deposit_params.token_amount);

        ctx.accounts.vault_authority.deposit_nonce += 1;

        let vault_deposit_params = VaultDepositParams {
            account_id: deposit_params.account_id,
            broker_hash: deposit_params.broker_hash,
            user_address: deposit_params.user_address, //
            token_hash: deposit_params.token_hash,
            src_chain_id: ctx.accounts.vault_authority.sol_chain_id,
            token_amount: deposit_params.token_amount as u128,
            src_chain_deposit_nonce: ctx.accounts.vault_authority.deposit_nonce,
        };

        emit!(Into::<VaultDeposited>::into(vault_deposit_params.clone()));

        let seeds = &[OAPP_SEED, &[ctx.accounts.oapp_config.bump]];

        let deposit_msg = VaultDepositParams::encode(&vault_deposit_params);
        let lz_message = LzMessage::encode(&LzMessage {
            msg_type: MsgType::Deposit as u8,
            payload: deposit_msg,
        });

        let options = EnforcedOptions::get_enforced_options(&ctx.accounts.enforced_options, &None);

        let endpoint_send_params = EndpointSendParams {
            dst_eid: ctx.accounts.vault_authority.dst_eid,
            receiver: ctx.accounts.peer.address,
            message: lz_message,
            options: options,
            native_fee: oapp_params.native_fee,
            lz_token_fee: oapp_params.lz_token_fee,
        };

        let receipt = oapp::endpoint_cpi::send(
            ctx.accounts.oapp_config.endpoint_program,
            ctx.accounts.oapp_config.key(),
            ctx.remaining_accounts,
            seeds,
            endpoint_send_params,
        )?;

        emit!(OAppSent {
            guid: receipt.guid,
            dst_eid: ctx.accounts.vault_authority.dst_eid,
        });

        Ok(receipt)
    }

Which the deposit params, will be first encoded in VaultDepositParams::encode, then combined with message type, altogether encoded again by LzMessage::encode, and sent to LZ endpoint. And in VaultDepositParams::encode:

    pub struct VaultDepositParams {
        pub account_id: [u8; 32],
        pub broker_hash: [u8; 32],
        pub user_address: [u8; 32],
        pub token_hash: [u8; 32],
        pub src_chain_id: u128,
        pub token_amount: u128,
        pub src_chain_deposit_nonce: u64,
    }

    pub fn encode(&self) -> Vec<u8> {
        let mut buf = Vec::new();
        buf.extend_from_slice(&self.account_id);
        buf.extend_from_slice(&self.broker_hash);
        buf.extend_from_slice(&self.user_address);
        buf.extend_from_slice(&self.token_hash);
        buf.extend_from_slice(&to_bytes32(&self.src_chain_id.to_be_bytes()));
        buf.extend_from_slice(&to_bytes32(&self.token_amount.to_be_bytes()));
        buf.extend_from_slice(&to_bytes32(&self.src_chain_deposit_nonce.to_be_bytes()));
        buf
    }

From the above function, we see from the struct, it should be encoded into a (32 * 7) = 224 bytes of data blob. For reference, here is the struct for AccountDepositSol, which will be decoded into:

struct AccountDepositSol {
    bytes32 accountId;
    bytes32 brokerHash;
    bytes32 userAddress;
    bytes32 tokenHash;
    uint256 srcChainId;
    uint128 tokenAmount;
    uint64 srcChainDepositNonce;
}

There is a discrepancy between the struct being encoded and decoded, and more likely, when such message is received by SolConnector, it will revert, as the unmarshal will fail.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

deposit function from LZ will fail due to discrepancy on structs, since for such message to send, tokens will be transferred first, but if the LZ message fails, those tokens will be lost.

PoC

No response

Mitigation

Adjust the struct, so that they align.