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

getRoundPhase() may lead to unintented result with the critical functions

bhunter

Summary

getRoundPhase is designed to calculate the current round, phase, and time remaining for a contract. It does so by iterating through a list of market parameters, marketParams, each associated with a specific timestamp. The issue with the iterating is that it assumes there is no discontinuity between consecutive market parameters, which may not be the case always. Any overlap or gap may lead to incorrect round, which may affect critical functions like purchase()

Vulnerability Details

getRoundPhase is defined as:

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.
​
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++;
}
​
//SNIP
​
return (round, phase, timeRemaining);
}
}

As it can be seen from the loop in the function, rounds are calculated using the difference in timestamps between consecutive market parameters in _computePhase() . _computePhase returns the round based on the integer division of elapsedTime / cycleTime. Since each phase calculation relies on elapsedTime, any discontinuity in marketParams[idx].timestamp values may result in incorrect round calculation.

Consider the following Cases:
Case 1: Suppose two market parameters have overlapping intervals, like so:

[
{ timestamp: 1000, sellInterval: 300, buyInterval: 200, withdrawInterval: 100 },
{ timestamp: 1200, sellInterval: 300, buyInterval: 200, withdrawInterval: 100 }
]

Here, the second marketParams entry starts at 1200, which falls within the timeframe of the first entry (1000 + cycleTime up to 1600). This causes ambiguity: should the function use the first or second parameter for phases within the overlapping time? This overlapping could result in skipped or repeated phases.

Case2: Assume the following timestamps with a gap between consecutive parameters:

[
{ timestamp: 1000, sellInterval: 300, buyInterval: 200, withdrawInterval: 100 },
{ timestamp: 2000, sellInterval: 300, buyInterval: 200, withdrawInterval: 100 }
]

In this case, the first marketParams entry ends at 1600, but the next one starts at 2000. This gap between 1600 and 2000 means there is no defined phase during this period. getRoundPhase could misinterpret the contract state if block.timestamp falls within this undefined gap. A potential
gap or overlap is highly likely to happen, because the market Params are set manually in the following function:

SwanManager.sol:
function setMarketParameters(SwanMarketParameters memory _marketParameters) external onlyOwner {
require(_marketParameters.platformFee <= 100, "Platform fee cannot exceed 100%");
_marketParameters.timestamp = block.timestamp;
marketParameters.push(_marketParameters);
}

As it can be seen block.timestamp is used as timestamp and it is set manually by the owner. If there is a delay between consecutive markets, then unintended gaps will exist.
Considering the round variable used in many place in the code, it may lead to undesired outcomes. For instance in purchase() function:

function purchase() external onlyAuthorized {
(uint256 round,) = _checkRoundPhase(Phase.Buy);
// check if the task is already processed
uint256 taskId = oraclePurchaseRequests[round];
if (isOracleRequestProcessed[taskId]) {
revert TaskAlreadyProcessed();
}
// read oracle result using the latest task id for this round
bytes memory output = oracleResult(taskId);
address[] memory assets = abi.decode(output, (address[]));
//SNIP
​
}

an incorrect round can lead to incorrect taskId, which may lead to revert and preventing the purchase.

Impact

Incorrect calculation of phase and round, potentially disrupting behavior of critical functions

Tools Used

Manual Review, Vs Code

Recommendations

Implement a logic that dynamically adjusts intervals to prevent gaps and overlaps.

Updates

Lead Judging Commences

inallhonesty Lead Judge
12 months ago
inallhonesty Lead Judge 12 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.