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

DoS-Like Effect on Admin Updates: Financial Impact from Parameter Synchronization

galturok

Summary

The current system design in the SwanManager and BuyerAgent contracts inadvertently allows for a Denial of Service (DoS)-like effect on the administrative ability to safely update market parameters such as platformFee and maxAssetCount. While changing the interval parameters (withdrawInterval, sellInterval, buyInterval) is known to disruptively increase the round count across all BuyerAgent instances and is intended by design, the issue arises when changes are made to the platformFee or maxAssetCount without altering any intervals. This situation causes all BuyerAgent instances to synchronize their phases uniformly, unbeknownst to users, leading to skipped Buy and Withdraw phases, thereby resulting in potential financial and operational losses.

Vulnerability Details

  • Location: The issue is concentrated within the setMarketParameters function of the SwanManager contract getRoundPhase of BuyerAgent.sol.

https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/swan/SwanManager.sol#L80C1-L84C6

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

https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/swan/BuyerAgent.sol#L334C1-L374C6

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

  • While altering any interval parameters is supposed to increase the round count, the vulnerability exploits the synchronization effect of updating non-interval properties (platformFee, maxAssetCount).

  • When the setMarketParameters function updates the platformFee or maxAssetCount, it uses the current block.timestamp, inadvertently affecting all buyer agents' phase calculations due to parameter timestamp updates.

  • As all agents use this timestamp to calculate their current phase, any modification leads to an unexpected reset and synchronizes all agents across the market to synchronize start a new phase together, bypassing critical user-interaction phases such as Buy and Withdraw.

Impact

This poses a significant challenge to the platform administrators who must update these parameters without unintentionally affecting the ongoing operations, leading to a form of operational gridlock.

Platform users and asset sellers may be detrimentally impacted as their listings can miss being processed in expected Buy phases, leading to potential unrecovered fee expenditures and lost sales opportunities.

Tools Used

Foundry

Recommendations

Consider implementing logic that differentiates timestamps affecting cycle calculations, allowing minor parameter changes (platformFee, maxAssetCount) to update without resetting the cycles or phases for existing agents.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Changes of the `maxAssetCount` and `platformFee` lead to lost funds due to round change

Appeal created

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

Changes of the `maxAssetCount` and `platformFee` lead to lost funds due to round change

Support

FAQs

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

Give us feedback!