code-423n4 / 2022-08-fiatdao-findings

2 stars 1 forks source link

Divide before multiply in slope calculations #230

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-fiatdao/blob/5a254ab15a387bd65a7dc50ac8371cb77de1e5d5/contracts/VotingEscrow.sol#L237-L242 https://github.com/code-423n4/2022-08-fiatdao/blob/5a254ab15a387bd65a7dc50ac8371cb77de1e5d5/contracts/VotingEscrow.sol#L245-L250

Vulnerability details

Impact

Dividing before doing multiplication in solidity can make the number to be rounded down. This is a threat as it can make a fee or something similar become 0, an unexpected result.

Proof of Concept

https://github.com/crytic/slither/wiki/Detector-Documentation#divide-before-multiply.

Tools Used

Manual analysis and slither.

Recommended Mitigation Steps

Just multiply before dividing as it will give the expected result.

lacoop6tu commented 2 years ago

Probably keeping this same as Curve and mStable

elnilz commented 2 years ago

this rounding issue doesn't seem to really impact the proper functioning of veFDT given this logic has been battle tested in Curve and a number of projects using Curve contracts over the years. that's why I downgrade severity to QA

gititGoro commented 2 years ago

Severity downgrade approved.

gititGoro commented 2 years ago

Duplicate of #244