Dria
Swan
NFTHardhat
21,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Asset lister can disapprove Swan contract from receiving their asset when trying to purchase()

elser17

Summary

Asset lister can prevent Swan contract from receiving their asset when trying to purchase()

Vulnerability Details

When a user calls Swan.list(), a new ERC721 token is deployed and 1 token is minted for the lister (which is ERC721 token's owner as well). The constructor automatically approves _operator address (which is Swan contract) to transfer that token from the lister when it is going to be purchased.

However, the lister can disapprove the Swan contract just after token deployment by calling this function from ERC721 asset.

setApprovalForAll(swanContract, false);

This way, when an authorized operator calls BuyerAgent.purchase(), the function will revert if any of the assets to be purchased got the approval revoked. This happens because BuyerAgent.purchase() loops all the decoded assets from BuyerAgent.oracleResult() and calls Swan.purchase() for each of them; Swan.purchase() tries to transferFrom the asset from the lister as it expects to be approved for so, but if approval is revoked as explained, then Swan.purchase() reverts.

Impact

If at least one of the assets that BuyerAgent intends to purchase gets the approval for Swan contract revoked, BuyerAgent will not be able to purchase any assets, leading to a DoS, as each of them is purchased individually with a loop. Even if the lister needs to transferRoyalties() to list an asset, the caused damage is much higher than its cost.

Tools Used

Manual review, Remix, ERC721 standard

Recommendations

The best idea to mitigate this issue is to modify SwanAsset so that lister can do nothing but wait until it is bought. Take into account that liste could revoke approval, send the token to another address, approve another address and transferFrom() the token from that address...

contract SwanAsset is ERC721, Ownable {
/// @notice Creation time of the token
uint256 public createdAt;
/// @notice Description of the token
bytes public description;
​
+ address public lister;
​
/// @notice Constructor sets properties of the token.
constructor(
string memory _name,
string memory _symbol,
bytes memory _description,
address _owner,
address _operator
) ERC721(_name, _symbol) Ownable(_owner) {
description = _description;
createdAt = block.timestamp;
+ lister = _owner;
// owner is minted the token immediately
ERC721._mint(_owner, 1);
​
// Swan (operator) is approved to by the owner immediately.
ERC721._setApprovalForAll(_owner, _operator, true);
}
​
+ modifier revertIfLister(){
+ if (msg.sender == lister){
+ revert();
+ }
+ _;
+ }
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Validated
Assigned finding tags:

DoS in BuyerAgent::purchase

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.