search

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

2 stars 1 forks source link

Performing multiplication on results of division #265

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

Solidity could truncate the results, performing multiplication before division will prevent rounding/truncation in solidity math.

Proof of Concept

https://github.com/corddry/ERC4626/blob/643cd044fac34bcbf64e1c3790a5126fec0dbec1/src/xERC4626.sol#L40

        rewardsCycleEnd = (block.timestamp.safeCastTo32() / rewardsCycleLength) * rewardsCycleLength;

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

        uint32 end = ((timestamp + rewardsCycleLength) / rewardsCycleLength) * rewardsCycleLength;

Tools Used

Slither + Manual Analysis

Recommended Mitigation Steps

Consider ordering multiplication first.

FortisFortuna commented 2 years ago

The truncation here is desired, so that the rewardCycleEnd goes down to the nearest integer multiple of the rewardsCycleLength.

joestakey commented 2 years ago

Could be a duplicate of #51 , though the warden does not explain how this could lead to an exploit.

0xean commented 2 years ago

closing as invalid, lacks impact