Attackers can steal any NFT that is currently under sale

[c4-bot-10]

Lines of code

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

Vulnerability details

Description

Any address that has approval can call execute.rs#transfer_nft() successfully because it ensures he has permissions using check_can_send(). Also if the address recipient has bids in this NFT it will send the funds to the previous owner

File: execute.rs

365:     fn transfer_nft(
/**CODE**/
370:         recipient: String,
371:         token_id: String,
372:     ) -> Result<Response<C>, ContractError> {
373:         let mut token = self.tokens.load(deps.storage, &token_id)?;
374:         // ensure we have permissions
375:         self.check_can_send(deps.as_ref(), &env, &info, &token)?;
 /**CODE**/
384:         for (i, item) in token.bids.iter().enumerate() {
385:             if item.address == recipient.to_string()
386:             {
387:                 position = i as i32;
388:                 amount = item.offer.into();
389:                 break;  
390:             }
...

When NFT is on sell with token.sell.auto_approve == true
any user can invoke execute.rs#setbidtobuy() two times and he will end up with an approval that never expired
Because the first call will user will transfer the funds to the contract in exchange for getting approval (so he can transfer the NFT).
However, the next call to setbidtobuy() will delete the user from the bids vec and send all the funds back to him. But it doesn't delete the approval in that second transaction

File: execute.rs#setbidtobuy()

693:         else {
694:             // update the approval list (remove any for the same spender before adding)
695:             token.bids.retain(|item| item.address != info.sender);
696:         }

Using this advantage attackers will take the approval and steal NFT with all the funds under it

Impact

Attackers can steal any NFT that is currently under sale.

Tools Used

Manual Review

Recommended Mitigation Steps

File: execute.rs#setbidtobuy()

693:         else {
694:             // update the approval list (remove any for the same spender before adding)
695:             token.bids.retain(|item| item.address != info.sender);
+                 token.approvals.retain(|apr| apr.spender != info.sender);
696:         }

Assessed type

Other

[c4-judge]

OpenCoreCH marked the issue as primary issue

[c4-judge]

OpenCoreCH marked the issue as duplicate of #12

[c4-judge]

OpenCoreCH marked the issue as satisfactory

[c4-judge]

OpenCoreCH marked the issue as not a duplicate

[c4-judge]

OpenCoreCH marked the issue as duplicate of #6