Dria
Swan
NFTHardhat
21,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

relist function Simply adds to new buyer's list without cleaning old state

kwakudr

Summary

relist function Simply adds to new buyer's list without cleaning old state

Vulnerability Details

function relist(address _asset, address _buyer, uint256 _price) external {
AssetListing storage asset = listings[_asset];
​
​
// only the seller can relist the asset
if (asset.seller != msg.sender) {
revert Unauthorized(msg.sender);
}
​
​
// asset must be listed
if (asset.status != AssetStatus.Listed) {
revert InvalidStatus(asset.status, AssetStatus.Listed);
}
​
​
// relist can only happen after the round of its listing has ended
// we check this via the old buyer, that is the existing asset.buyer
//
// note that asset is unlisted here, but is not bought at all
//
// perhaps it suffices to check `==` here, since buyer round
// is changed incrementially
(uint256 oldRound,,) = BuyerAgent(asset.buyer).getRoundPhase();
if (oldRound <= asset.round) {
revert RoundNotFinished(_asset, asset.round);
}
​
​
// now we move on to the new buyer
BuyerAgent buyer = BuyerAgent(_buyer);
(uint256 round, BuyerAgent.Phase phase,) = buyer.getRoundPhase();
​
​
// buyer must be in sell phase
if (phase != BuyerAgent.Phase.Sell) {
revert BuyerAgent.InvalidPhase(phase, BuyerAgent.Phase.Sell);
}
​
​
// buyer must not have more than `maxAssetCount` many assets
uint256 count = assetsPerBuyerRound[_buyer][round].length;
if (count >= getCurrentMarketParameters().maxAssetCount) {
revert AssetLimitExceeded(count);
}
​
​
// create listing
listings[_asset] = AssetListing({
createdAt: block.timestamp,
royaltyFee: buyer.royaltyFee(),
price: _price,
seller: msg.sender,
status: AssetStatus.Listed,
buyer: _buyer,
round: round
});
​
​
// add this to list of listings for the buyer for this round
assetsPerBuyerRound[_buyer][round].push(_asset);
​
​
// transfer royalties
transferRoyalties(listings[_asset]);
​
​
emit AssetRelisted(msg.sender, _buyer, _asset, _price);
}

AssetListing storage asset = listings[_asset];

The variable name asset shadows the parameter _asset, making the code confusing and error-prone

// Example Scenario:

// Initial state:

assetsPerBuyerRound[buyerA][1] = [asset1, asset2, asset3]

// After relisting asset2 to buyerB:

assetsPerBuyerRound[buyerA][1] = [asset1, asset2, asset3] // Still contains asset2!

assetsPerBuyerRound[buyerB][2] = [asset2] // Added here too

// Problems:

// 1. Asset counted towards buyerA's limit

// 2. Inconsistent state

Impact

Inconsistent state in relist which affects maxAssetCount calculations. Doesn't remove asset from old buyer's list

Tools Used

Manual Review

Recommendations

Find asset in old buyer's array. Remove asset efficiently. Move last element to the removed position (unless it's the last element)

Updates

Lead Judging Commences

inallhonesty Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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