Description

The AuthorizeGovernanceDataSourceTransfer action is used to modify the currently authorized governance source (i.e., the caller address that may perform governance actions through this contract). This is done through the execute_governance_instruction() function.

Specifically, the AuthorizeGovernanceDataSourceTransfer calls into transfer_governance(). This action allows the caller (who is the currently authorized governance source) to pass in a claim VAA that contains information about the new governance source to authorize. This claim VAA is supplied by the new governance source.

To prevent replay attacks, the claim VAA also contains a governance_data_source_index, which needs to be larger than the currently stored index. If it is not, it means that a previous AuthorizeGovernanceDataSourceTransfer message is being replayed, and thus the contract will reject it. This check can be seen in the transfer_governance() function:

fn transfer_governance(
    next_config: &mut ConfigInfo,
    current_config: &ConfigInfo,
    parsed_claim_vaa: &ParsedVAA,
) -> StdResult<Response> {
    // [ ... ]

    match claim_vaa_instruction.action {
        RequestGovernanceDataSourceTransfer {
            governance_data_source_index,
        } => {
            if current_config.governance_source_index >= governance_data_source_index {
                Err(PythContractError::OldGovernanceMessage)?
            }

            // [ ... ]
        }
        _ => Err(PythContractError::InvalidGovernancePayload)?,
    }
}

The governance_source_index configuration property is a u32, so if the new governance source passes in a RequestGovernanceDataSourceTransfer action with the governance_data_source_index property set to the maximum u32 value, then any subsequent RequestGovernanceDataSourceTransfer action can never have a higher governance_data_source_index property, and thus this action can never be performed again.

Impact

We do not consider this a security issue, as the new governance source is considered to be a trusted entity by the protocol already. We do however recommend that this be fixed, as the new governance source may accidentally brick this governance source transfer functionality of the contract by passing in the maximum u32 value for the governance_data_source_index.

Recommendations

Consider adding a check such that the governance_data_source_index is higher than the currently stored governance_source_index but still within a certain amount.

Remediation

Pyth Data Association acknowledges the finding and developed a patch for this issue: commit 3e104b41↗.