pyth2wormhole - Governance

In the program `staking`, the role `governance_authority` has authority over the following functions: - `update_governance_authority()` will change the `governance_authority` role of the `config_account`. - `update_freeze()` will change the `freeze` status of the `config_account`. - `create_target()` will create a voting target `target_account`. Any compromise to the `governance_authority` account may allow the hacker to take advantage of this and result in unexpected loss. Additionally, the Solana program could be upgradeable, and the upgrade authority is the deployer by default. Therefore, if the program is upgradable, and the upgrade authority account is compromised, it could lead to a malicious program upgrade, thus introducing centralization risk.

The above linked locations are places where checked math is not used and integer overflow is possible.

In L43, the program is using a default program ID provided by the Anchor framework: ```rust=43 declare_id!("Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS"); ```

In the program `staking`, the functions that affect the status of sensitive variables should be able to emit events. - `init_config` affects the `GlobalConfig` account - `update_governance_authority` affects `governance_authority` of the `GlobalConfig` account - `update_freeze` affects `freeze` of the `GlobalConfig` account - `create_stake_account` affects `StakeAccountMetadataV2` account and `VoterWeightRecord` account - `create_position` affects `PositionData` account - `close_position` affects `PositionData` account - `update_voter_weight` affects `VoterWeightRecord` account

The **#[account(address = <expr>)]** in Anchor supports throwing an exception if the address does not match. It is better to use **#[account(address = <expr> @ custom_error)]** instead of **#[account(address = <expr>)]** to provide a customized error message and pass it back to the caller. Reference:[Accounts in anchor_lang](https://docs.rs/anchor-lang/latest/anchor_lang/derive.Accounts.html)

When withdrawing the staked token, a `destination` TokenAccount is given but missing the mint check against `config.pyth_token_mint`.

The following two risk validations at L322 to L332 and L345 to L355 in the `withdraw_stake` instruction are redundant. ```rust=318 let remaining_balance = stake_account_custody .amount .checked_sub(amount) .ok_or_else(|| error!(ErrorCode::InsufficientWithdrawableBalance))?; if utils::risk::validate( stake_account_positions, remaining_balance, unvested_balance, current_epoch, config.unlocking_duration, ) .is_err() { return Err(error!(ErrorCode::InsufficientWithdrawableBalance)); } transfer( CpiContext::from(&*ctx.accounts).with_signer(&[&[ AUTHORITY_SEED.as_bytes(), ctx.accounts.stake_account_positions.key().as_ref(), &[stake_account_metadata.authority_bump], ]]), amount, )?; ctx.accounts.stake_account_custody.reload()?; if utils::risk::validate( stake_account_positions, ctx.accounts.stake_account_custody.amount, unvested_balance, current_epoch, config.unlocking_duration, ) .is_err() { return Err(error!(ErrorCode::InsufficientWithdrawableBalance)); } ```

When updating a voter's weight with the `CastVote` action, a proposal account is deserialized, but not sufficiently checked. For example, since the `governance` and `governing_token_mint` fields are not checked, it is possible for the proposal account to belong to different governance. Furthermore, the `state` field is not checked, meaning the proposal may have already succeeded or failed.

When creating a stake account, the user provides the vesting schedule. If `PeriodVesting` is chosen, four parameters need to be provided: - `initial_balance` - `start_date` - `period_duration` - `num_periods` Since there are operations that involve dividing by `period_duration` or `num_periods`, there should be checks to ensure that these parameters are positive, especially since they cannot be changed after the stake account is created.