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

Uncontrolled Offset rounds in `BuyerAgent::getRoundPhase` could force sellers to relist

Sickurity

Summary

Issue is quite complex. Let’s go in order.

  1. After reviewing the function BuyerAgent::purchase, it becomes clear that it only makes a purchase in the round which gives the function BuyerAgent::getRoundPhase.Moreover buyer can buy only on BUY phase when users can create listings only on SELL phase.

  2. The BuyerAgent::getRoundPhase function calculates the number of rounds for each marketParametrs value in the Swan. The sum is the current round.

  3. MarketParametrs are the Swan Market parameters that Swan Owner sets in SwanManager::setMarketParameters. All market parameters are added to the array and can no longer be removed from it.

  4. For each market parameter, a so-called offset round is added to the getRoundPhase function.

Key attack idea - because of offset getRoundPhase can skip from round n SELL phase to round n + 1 SELL phase. So round n BUY phase will skipped, and users with lists on round n, that can be bought only on round n BUY phase, will be forced to relist and pay more txes

Vulnerability Details

Let the current round be - n. And in this round the owner will decide to add new marketParametrs.

If we were in (n, SELL) and owner add new market parameters we will move straight to (n + 1, SELL). Because of offset in last parametr round computation

From the same reasoning (n, BUY) -> (n + 1, SELL). (n, WITHDRAW) -> (n + 1, SELL)

function getRoundPhase() public view returns (uint256, Phase, uint256) {
SwanMarketParameters[] memory marketParams = swan.getMarketParameters();
​
if (marketParams.length == marketParameterIdx + 1) {
// if our index is the last market parameter, we can simply treat it as a single instance,
// and compute the phase according to the elapsed time from the beginning of the contract.
return _computePhase(marketParams[marketParameterIdx], block.timestamp - createdAt);
} else {
// we will accumulate the round from each phase, starting from the first one.
uint256 idx = marketParameterIdx;
//
// first iteration, we need to compute elapsed time from createdAt:
// createdAt -|- VVV | ... | ... | block.timestamp
(uint256 round,,) = _computePhase(marketParams[idx], marketParams[idx + 1].timestamp - createdAt);
idx++;
// start looking at all the intervals beginning from the respective market parameters index
// except for the last element, because we will compute the current phase and timeRemaining for it.
​
// @audit for simplicity we can assume that sum of this rounds is n, because we receive this sum before
while (idx < marketParams.length - 1) {
// for the intermediate elements we need the difference between their timestamps:
// createdAt | ... -|- VVV -|- ... | block.timestamp
(uint256 innerRound,,) =
_computePhase(marketParams[idx], marketParams[idx + 1].timestamp - marketParams[idx].timestamp);
​
// accumulate rounds from each intermediate phase, along with a single offset round
round += innerRound + 1;
​
idx++;
}
​
// for last element we need to compute current phase and timeRemaining according
// to the elapsedTime at the last iteration, where we need to compute from the block.timestamp:
// createdAt | ... | ... | VVV -|- block.timestamp
(uint256 lastRound, Phase phase, uint256 timeRemaining) =
_computePhase(marketParams[idx], block.timestamp - marketParams[idx].timestamp);
// accumulate the last round as well, along with a single offset round
// @audit right after marketParametrs adding lastRound = 0, but offset is 1, so returned round is n + 1
round += lastRound + 1;
​
return (round, phase, timeRemaining);
}

Although in each case this discrete jump slightly disrupts the protocol, it is only in specific cases that it directly threatens the user’s funds.

  1. From (n, SELL) to (n + 1, SELL)

In this case all users that listed they assets on n round simply will not have the ability to sell their asset. Since a buyer can only buy these sets in (n, BUY) however this phase will be skipped. Thus users will be forced to relist their assets and additionally pay a commission not on their own fault.

2.From (n, BUY) to (n + 1, SELL) before Buyer purchase. Consequense is same as in the previous case, but now buyer just dont call purchase in time to trigger a transaction before updating the parametrs.(It is possible because buyers dont call immideatly after BUY phase starts)

Impact

When the user adds new parameters - 50/50 chance that will happen situation described above. As a result, users who put up an asset in a certain round are forced to re-put it and pay a new fee.

Severity: Medium

Tools Used

Manual Review

Recommendations

Better controll offset addition in round calculation

Updates

Lead Judging Commences

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Known issue

Appeal created

bengalcatbalu Submitter
8 months ago
bengalcatbalu Submitter
8 months ago
inallhonesty Lead Judge
7 months ago
inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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