Multiplication performed after division can truncate the results

[code423n4]

Lines of code

https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/FraxlendPairCore.sol#L307-L316 https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/FraxlendPairCore.sol#L409-L497 https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/FraxlendPairCore.sol#L911-L942 https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/FraxlendPairCore.sol#L950-L1032 https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/libraries/VaultAccount.sol#L16-L29 https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/libraries/VaultAccount.sol#L33-L46 https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/LinearInterestRate.sol#L76-L92 https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/VariableInterestRate.sol#L63-L85

Vulnerability details

Multiplication performed after division can truncate the results

Impact

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

Details

This can affect variables such as slopes, interests, fees, shares and all kinds of amounts.

Proof of Concept

https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/FraxlendPairCore.sol#L307-L316 https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/FraxlendPairCore.sol#L409-L497 https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/FraxlendPairCore.sol#L911-L942 https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/FraxlendPairCore.sol#L950-L1032 https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/libraries/VaultAccount.sol#L16-L29 https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/libraries/VaultAccount.sol#L33-L46 https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/LinearInterestRate.sol#L76-L92 https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/VariableInterestRate.sol#L63-L85

Recommended Mitigation Steps

Reorder the operations. For more info of the issue: https://github.com/crytic/slither/wiki/Detector-Documentation#divide-before-multiply

[DrakeEvans]

Intended, meant to protect from overflow. Loss of precision is minimal.

[gititGoro]

When dividing by constants that have the word precision in the name, it's often intended behaviour on the part of the dev. Wardens are encouraged to either consult documentation or contact warden. If those channels don't work then the best case scenario is to submit as a QA issue. Because precision appears in the names of the constants, I'm going to fall on the side of marking this invalid. If the denominators were variables or constants named with the intention of not truncating then I'd downgrade to QA.