Dria

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

Malicious Owner Can Create High Value Asset & Revoke Swan Approval Of Asset To Not Make A Single Purchase

Summary

A malicous user can create a high value asset and list to r a buyer and then revoke the swan contract approval to transfer the token Id to the buyer in turn denying every other lister a chance to earn from the purchase in that round.

This could also be a malicous owner who could create a high value asset and list it to himself with another address. Then revoke approval for the token Id. In order to gain all the listing fees from listers without eventually having to make a purchase from the round due to the lack of approval during the transfer of the asset.

Vulnerability Details

This could be another user but I'm using a malicious buyer as an example because he stands to gain all the lsiting fees without actually making a purchase.

When a malicious buyer list a high value created asset to himself. Other sellers list to the buyer as well, the buyer earns royalties from these listers. Now the buyers asset being a high value asset is picked as one of the assets returned to be purchased when the oraclePurchaseRequest function is called, task is completed and output returned.

Before or as soon as the oraclePurchaseRequest is in progress, the malicous buyer then revokes the approval of the Swan for the tokenId that was set on the asset creation, the buyer does this using the setApprovalForAllfunction in the asset, being an ERC721.

Here is where the approval was set for the SwanAsset for all the tokens. The owner can eventually revoke this approval with the setApprovalForAll with a false value at any time after the assets creation.

constructor(
string memory _name,
string memory _symbol,
bytes memory _description,
address _owner,
address _operator
) ERC721(_name, _symbol) Ownable(_owner) {
description = _description;
createdAt = block.timestamp;
// owner is minted the token immediately
ERC721._mint(_owner, 1);
// Approval set for the swan contract for all tokens
ERC721._setApprovalForAll(_owner, _operator, true);
}

Once this approval has been revoked and the high value asset the malicious owner listed to himself makes it to the encoded addresses array output. The purchase function would always revert in the buyerAgent. This is because the purchase function loops through all the assets that are to be bought and the Swan contract performs a transferFrom on those assets to the buyer agent. See code below in the buyerAgent

function purchase() external onlyAuthorized {
// .... some code here ......
bytes memory output = oracleResult(taskId);
// high value asset included in the address array of asset to be bought
address[] memory assets = abi.decode(output, (address[]));
// once it gets to the high value assets turn it reverts
for (uint256 i = 0; i < assets.length; i++) {
address asset = assets[i];
uint256 price = swan.getListingPrice(asset);
spendings[round] += price;
if (spendings[round] > amountPerRound) {
revert BuyLimitExceeded(spendings[round], amountPerRound);
}
inventory[round].push(asset);
// make the actual purchase in the swan contract
// which tries to transfer the asset from the owner to the buyer
// that'll fail due to the malicous owner revoking approval
swan.purchase(asset);
}
isOracleRequestProcessed[taskId] = true;
}

The transferFrom would revert when it gets to the high value asset because it no longer has the approval it requires to make the transfer due to the malicous owners high value asset that was revoked being a part of the asset. See code below in the Swan contract

function purchase(address _asset) external {
// .... some code here .....
// this statement would revert due to lack of approval by the malicous owner
SwanAsset(_asset).transferFrom(listing.seller, address(this), 1);
SwanAsset(_asset).transferFrom(address(this), listing.buyer, 1);
token.transferFrom(listing.buyer, address(this), listing.price);
token.transfer(listing.seller, listing.price);
emit AssetSold(listing.seller, msg.sender, _asset, listing.price);
}

Impact

The malicous owner get all the royalties paid by all the listers to him but does not make a single purchase of any listed items whether its called by the oracle or the owner themselves, the purchase function would simply revert.

Recommendations

Instead of using approvals that can be revoked at any time, transfer and store the assets in the contract on listing and attach them to owner providing a way for the owner to be able to withdraw the asset if its not sold. That way the contract has full control of the listed asset

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.