Summary
Failing to adjust the timestamp to the current block time during Swan contract initialization may result in skewed or broken phase calculations within the BuyerAgent contract.
Vulnerability Details
As it is stated about SwanMarketParameters.timestamp:
Even if this is provided by the user, it will get overwritten by the internal block.timestamp
https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/swan/SwanManager.sol#L26-L26
struct SwanMarketParameters {
uint256 withdrawInterval;
uint256 sellInterval;
uint256 buyInterval;
uint256 platformFee;
uint256 maxAssetCount;
-->
uint256 timestamp;
}
Indeed, in the SwanManager contract owner-controlled setMarketParameters method overrides the timestamp value:
https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/swan/SwanManager.sol#L82
function setMarketParameters(SwanMarketParameters memory _marketParameters) external onlyOwner {
require(_marketParameters.platformFee <= 100, "Platform fee cannot exceed 100%");
_marketParameters.timestamp = block.timestamp;
marketParameters.push(_marketParameters);
}
However, it is failing to do the same in the Swan::initialize:
https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/swan/Swan.sol#L132-L132
function initialize(
SwanMarketParameters calldata _marketParameters,
LLMOracleTaskParameters calldata _oracleParameters,
address _coordinator,
address _token,
address _buyerAgentFactory,
address _swanAssetFactory
) public initializer {
__Ownable_init(msg.sender);
require(_marketParameters.platformFee <= 100, "Platform fee cannot exceed 100%");
marketParameters.push(_marketParameters);
oracleParameters = _oracleParameters;
coordinator = LLMOracleCoordinator(_coordinator);
token = ERC20(_token);
buyerAgentFactory = BuyerAgentFactory(_buyerAgentFactory);
swanAssetFactory = SwanAssetFactory(_swanAssetFactory);
isOperator[address(this)] = true;
isOperator[msg.sender] = true;
}
This may lead to incorrect calculations and potential reverts due to arithmetic underflow during the execution of BuyerAgent::getRoundPhase() if the user-provided value is zero or set to an arbitrarily high value.
Specifically, in the snippet below:
-
line #347 will revert, if, for example, a caller didn't set timestamp (timestamp is 0), assuming it will be set to block.timestamp by the contract.
-
line #356 will revert, if a caller set it to arbitrary high value.
-
even in the case, when calculations will not revert, it could lead to incorrect phase calculations by _computePhase as can be seen at the same lines of code.
334 function getRoundPhase() public view returns (uint256, Phase, uint256) {
335 SwanMarketParameters[] memory marketParams = swan.getMarketParameters();
336
337 if (marketParams.length == marketParameterIdx + 1) {
338
339
340 return _computePhase(marketParams[marketParameterIdx], block.timestamp - createdAt);
341 } else {
342
343 uint256 idx = marketParameterIdx;
344
345
346
347 (uint256 round,,) = _computePhase(marketParams[idx], marketParams[idx + 1].timestamp - createdAt);
348 idx++;
349
350
351
352 while (idx < marketParams.length - 1) {
353
354
355 (uint256 innerRound,,) =
356 _computePhase(marketParams[idx], marketParams[idx + 1].timestamp - marketParams[idx].timestamp);
357
358
359 round += innerRound + 1;
360
361 idx++;
362 }
363
364
365
366
367 (uint256 lastRound, Phase phase, uint256 timeRemaining) =
368 _computePhase(marketParams[idx], block.timestamp - marketParams[idx].timestamp);
369
370 round += lastRound + 1;
371
372 return (round, phase, timeRemaining);
373 }
374 }
Impact
The contract deployer may be misled into believing that the provided timestamp does not matter and will be overridden, which is false. An incorrect timestamp value may lead to arithmetic overflow, breaking the protocol or resulting in incorrect phase calculations, thereby skewing its logic.
Tools Used
Manual review
Recommendations
Add the following line into the Swan::initialize before pushing this variable into marketParameters array.
_marketParameters.timestamp = block.timestamp;
