code-423n4 / 2022-02-anchor-findings

0 stars 0 forks source link

[WP-H2] `money-market-contracts/contracts/market` `claim_rewards` may revert due to `spend_limit` set on `distributor` #46

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/money-market-contracts/contracts/market/src/borrow.rs#L216-L234

Vulnerability details

While claim_rewards from the money-market, it calls the distributor_contract#spend() to send the rewards.

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/money-market-contracts/contracts/market/src/borrow.rs#L216-L234

let messages: Vec<CosmosMsg> = if !claim_amount.is_zero() {
    vec![CosmosMsg::Wasm(WasmMsg::Execute {
        contract_addr: deps
            .api
            .addr_humanize(&config.distributor_contract)?
            .to_string(),
        funds: vec![],
        msg: to_binary(&FaucetExecuteMsg::Spend {
            recipient: if let Some(to) = to {
                to.to_string()
            } else {
                borrower.to_string()
            },
            amount: claim_amount.into(),
        })?,
    })]
} else {
    vec![]
};

However, the distributor_contract#spend() function have a spend_limit config and it will revert if the amount is larger than the spend_limit.

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/anchor-token-contracts/contracts/distributor/src/contract.rs#L153-L155

if config.spend_limit < amount {
    return Err(StdError::generic_err("Cannot spend more than spend_limit"));
}

As a result, users won't be able to claim their rewards anymore once the amount of the rewards excess the spend_limit config on distributor_contract.

Recommendation

Consider removing the spend_limit or allowing users to specify an amount when claim_rewards.

GalloDaSballo commented 2 years ago

Loss of yield, conditional on reaching limit, consider reducing severity

albertchon commented 2 years ago

Can be mitigated with a large spend limit, which likely exists to prevent catastrophic cases. The issue is correct though, that this is a bug. Downgrading to med severity though, given the practicality.