Attacker can drain all the funds in this contract

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-10-coded-estate/blob/main/contracts/codedestate/src/execute.rs#L708

Vulnerability details

Description

Every NFT has this pub sell: Sell field in its TokenInfo struct which is for selling NFTs

pub struct Sell {
    pub islisted: Option<bool>,
    pub auto_approve: bool,
    pub price: u64,
    pub denom: String,
}

This contract is used by multiple homeowners which means it holds the funds (from the reservation and bid) for all the NFT in one place.
NFT owner can update the pub sell: Sell values by triggering execute.rs#setlistforsell()

    pub fn setlistforsell(
    /***CODE***/
    ) -> Result<Response<C>, ContractError> {

        let mut token = self.tokens.load(deps.storage, &token_id)?;
        // ensure we have permissions
        self.check_can_approve(deps.as_ref(), &env, &info, &token)?;

        token.sell.islisted = Some(islisted);
        token.sell.price = price;
        token.sell.auto_approve = auto_approve;
        token.sell.denom = denom;
        self.tokens.save(deps.storage, &token_id, &token)?;

Due to the lack of checking the current state of the NFT in this function address with permissions can update the values at any time.
Malicious users can take this advantage to steal all the funds (native token NIBI + all tokens from IBC ) by:
1- Mint NFT and set it for sell, set the token.sell.denom to IBC token that has a dust value in the market
2- Then call setbidtobuy() function to set offer:info.funds[0].amount this amount will stay in the smart contract
3- Now, re-set token.sell.denom (by invoking setlistforsell()) to a token that has more value (or even fewer decimals)
4- Back to setbidtobuy() call it, this will lead to draining all the funds (of the specific token) in this cw721 smart contract

this happens because this message takes denom from the sell denom: token.sell.denom which can be changed at any time.

            .add_message(BankMsg::Send {
                to_address: info.sender.to_string(),
                amount: vec![Coin {
                    denom: token.sell.denom,
                    amount: amount,
                }],
            }))

Impact

An attacker can drain all the funds in this contract

Manual Review

Recommended Mitigation Steps

Do not allow to update denom if pub bids: Vec<Bid> is not empty

Assessed type

Invalid Validation

[c4-judge]

OpenCoreCH marked the issue as duplicate of #5

[c4-judge]

OpenCoreCH marked the issue as satisfactory