search

code-423n4 / 2022-09-frax-findings

2 stars 1 forks source link

[M1] 32-bit timestamp usage will make contract unusable in 2106 #336

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/corddry/ERC4626/blob/643cd044fac34bcbf64e1c3790a5126fec0dbec1/src/xERC4626.sol#L80 https://github.com/corddry/ERC4626/blob/643cd044fac34bcbf64e1c3790a5126fec0dbec1/src/xERC4626.sol#L89 https://github.com/corddry/ERC4626/blob/643cd044fac34bcbf64e1c3790a5126fec0dbec1/src/xERC4626.sol#L27 https://github.com/corddry/ERC4626/blob/643cd044fac34bcbf64e1c3790a5126fec0dbec1/src/xERC4626.sol#L30

Vulnerability details

Impact

Contract will stop working in 2106 because of timestamp overflow and revert

PoC

It is recommended to use timestamps with more bits. Using 48 bits, will make this contract work until around year 9,000,000 !! so it is worth the small change. Besides using 32 bits will not provide any advantage unless you pack storage variables .
Finally, as this contract is not upgradeable it will just stop working and you will never be able to fix it in the future.

Recommended

Change the following variables to 48 which will last millions of years

[-] uint32 public lastSync;  
[+] uint48 public lastSync; 

[-] uint32 public rewardsCycleEnd;  
[+] uint48 public rewardsCycleEnd; 

[-] uint32 timestamp = block.timestamp.safeCastTo32();  
[+] uint48 timestamp = block.timestamp.safeCastTo48();
FortisFortuna commented 2 years ago

We would replace this contract by then

peritoflores commented 2 years ago

Hi @FortisFortuna, regarding the severity of this issue. There has been arguments about some similar issues when the likelihood of something makes an issue medium or QA. For example the following issue is very similar

https://code4rena.com/reports/2022-06-nibbl/
[[M-02] TWAV.SOL#_GETTWAV() WILL REVERT WHEN TIMESTAMP > 4294967296] (https://github.com/code-423n4/2022-06-nibbl-findings/issues/178)

was judged as medium risk.

I leave this decision to judges but I still believe that this should be medium to be consistent with previous findings. Finally, as I mentioned before this contract is not upgradeable. An upgradeable contract will make this issue probably a QA.

Again these are just my arguments and I leave decision to judges.

0xean commented 2 years ago

closing as invalid, QA at best..