code-423n4 / 2023-10-ethena-findings

5 stars 5 forks source link

Error in if operation causes malfunction in all contract logic #40

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-10-ethena/blob/ee67d9b542642c9757a6b826c82d0cae60256509/contracts/StakedUSDe.sol#L176 https://github.com/code-423n4/2023-10-ethena/blob/ee67d9b542642c9757a6b826c82d0cae60256509/contracts/StakedUSDe.sol#L94 https://github.com/code-423n4/2023-10-ethena/blob/ee67d9b542642c9757a6b826c82d0cae60256509/contracts/StakedUSDe.sol#L166

Vulnerability details

Impact

The operation performed in if(timeSinceLastDistribution >= VESTING_PERIOD) should be the other way around, since timeSinceLastDistribution = block.timestamp - lastDistributionTimestamp;

Proof of Concept

therefore example:

Therefore, it will be allowed to withdraw assets without the need to respect the vesting.

In contrast, if no operation is performed to modify lastDistributionTimestamp and 8 hours pass, the following will happen:

and also the function totalAssets(L166) will give an incorrect value of the state.

The L180 VESTING_PERIOD - timeSinceLastDistribution operation is possibly also wrong.

Recommended Mitigation Steps

In L176 the following if (timeSinceLastDistribution <= VESTING_PERIOD) should be performed.

Assessed type

Invalid Validation

raymondfam commented 1 year ago

Everything is working as intended. (timeSinceLastDistribution <= VESTING_PERIOD) would make totalAssets() release all vested tokens though, facilitating users frontrunning the payment of yield.

c4-pre-sort commented 1 year ago

raymondfam marked the issue as low quality report

c4-pre-sort commented 1 year ago

raymondfam marked the issue as primary issue

c4-judge commented 1 year ago

fatherGoose1 marked the issue as unsatisfactory: Invalid