Anchor contest - IllIllI's results

Lines of code

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/anchor-token-contracts/contracts/staking/src/contract.rs#L331-L343

Vulnerability details

Impact

Distribution schedules can have s.0 and s.0 be in the wrong order leading to negative numbers, and thus negative distribution amounts per second

Proof of Concept

    for s in config.distribution_schedule.iter() {
        if s.0 > block_time || s.1 < state.last_distributed {
            continue;
        }

        // min(s.1, block_time) - max(s.0, last_distributed)
        let passed_time =
            std::cmp::min(s.1, block_time) - std::cmp::max(s.0, state.last_distributed);

        let time = s.1 - s.0;
        let distribution_amount_per_second: Decimal = Decimal::from_ratio(s.2, time);
        distributed_amount += distribution_amount_per_second * Uint128::from(passed_time as u128);
    }

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/anchor-token-contracts/contracts/staking/src/contract.rs#L331-L343

Tools Used

Code inspection

Recommended Mitigation Steps

The vesting schedules don't have this issue because they check the order:

fn assert_vesting_schedules(vesting_schedules: &[(u64, u64, Uint128)]) -> StdResult<()> {
    for vesting_schedule in vesting_schedules.iter() {
        if vesting_schedule.0 >= vesting_schedule.1 {
            return Err(StdError::generic_err(
                "end_time must bigger than start_time",
            ));
        }
    }

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/anchor-token-contracts/contracts/vesting/src/contract.rs#L91-L98

so the distribution schedules should have similar checks

Don't parse and generate strings to get the fractional part of Decimals

fn get_decimals(value: Decimal) -> StdResult<Decimal> {
    let stringed: &str = &*value.to_string();
    let parts: &[&str] = &*stringed.split('.').collect::<Vec<&str>>();
    match parts.len() {
        1 => Ok(Decimal::zero()),
        2 => {
            let decimals = Decimal::from_str(&*("0.".to_owned() + parts[1]))?;
            Ok(decimals)
        }
        _ => Err(StdError::generic_err("Unexpected number of dots")),
    }
}

https://github.com/code-423n4/2022-02-anchor/blob/main/contracts/anchor-bAsset-contracts/contracts/anchor_basset_reward/src/user.rs#L227-L238 Do what normal rust Decimal does instead: https://docs.rs/rust_decimal/0.7.2/src/rust_decimal/decimal.rs.html#353-357 https://docs.rs/rust_decimal/0.7.2/src/rust_decimal/decimal.rs.html#315-332

Refactor common code

These four blocks do the same creation of increase/decrease messages. They should be refactored to a common function to save on deployment gas https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/anchor-bEth-contracts/contracts/anchor_beth_token/src/handler.rs#L33-L50 https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/anchor-bEth-contracts/contracts/anchor_beth_token/src/handler.rs#L122-L145 https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/anchor-bEth-contracts/contracts/anchor_beth_token/src/handler.rs#L168-L185 https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/anchor-bEth-contracts/contracts/anchor_beth_token/src/handler.rs#L236-L256

Move most frequently used messages to beginning of match statements

These are some examples where the most frequently matched message isn't checked first. Rearranging should save gas https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/anchor-bAsset-contracts/contracts/anchor_basset_token/src/contract.rs#L55-L87 https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/anchor-bAsset-contracts/contracts/anchor_basset_reward/src/contract.rs#L51-L61 https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/anchor-bAsset-contracts/contracts/anchor_basset_reward/src/contract.rs#L66-L73 https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/anchor-bAsset-contracts/contracts/anchor_basset_hub/src/contract.rs#L115-L185 https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/anchor-bAsset-contracts/contracts/anchor_basset_hub/src/contract.rs#L435-L447 https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/anchor-bEth-contracts/contracts/anchor_beth_token/src/contract.rs#L50-L83 https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/anchor-bEth-contracts/contracts/anchor_beth_reward/src/contract.rs#L59-L80