code423n4 commented 3 years ago

Handle

heiho1

Vulnerability details

Impact

DaoVault.withdraw(address,address) uses block.timestamp based comparisons can be affected by miner behavior, leading to withdrawal impacts on the user.

Proof of Concept

https://github.com/code-423n4/2021-07-spartan/blob/e2555aab44d9760fdd640df9095b7235b70f035e/contracts/DaoVault.sol#L69

Tools Used

Slither

Recommended Mitigation Steps

An external time oracle like ChainLink Alarm Clock is worth consideration: https://blog.chain.link/blockchain-voting-using-a-chainlink-alarm-clock-oracle/

verifyfirst commented 3 years ago

Oracles are not in our scope. We find issues with timestamps invalid

SamusElderg commented 3 years ago

Not possible to manipulate backwards; hence the deposit time can only be manipulated to a detriment so we can rule that out of the equation here.

So in this case the manipulation would have to be >= 86400 during DaoVault.withdraw to cause concern. The scope of this manipulation is even less on fast blockchains; on BSC we are talking one or two blocks (less than 10 seconds).

Whilst we understand block.timestamp is not a true source of 100% truth; the intention was to only use in areas where 100% accuracy is not required. Would be very different if we were just trying to prevent deposit -> withdraw in the same block (flashloans and the similar) however, the above example only needs to deter flashloan manipulation and bad-actors trying to extract value over the short-term, which continues to do despite the block.timestamp manipulation potential. So this is a non-issue based on the intended functionality in my opinion.

ghoul-sol commented 3 years ago

per sponsor comment, invalid

code-423n4 / 2021-07-spartan-findings