sherlock-audit / 2024-04-titles-judging

9 stars 6 forks source link

Enc3yptedDegen - Faulty Division Operation that could lead to potential logical errors. #227

Closed sherlock-admin4 closed 4 months ago

sherlock-admin4 commented 4 months ago

Enc3yptedDegen

high

Faulty Division Operation that could lead to potential logical errors.

Summary

In Solidity, as in many programming languages, multiplication and division have the same precedence and are evaluated from left to right. This report aims to analyze the warning, its implications, and the recommended solution to ensure the correct execution of the smart contract.

Vulnerability Detail

A division operation precedes multiplication operations in the code, which could lead to unexpected results due to the way integer division truncates in Solidity. The specific line of code causing this is: *_protocolShare = protocolFee protocolFeeshareBps / MAXBPS;**

Impact

Any division operation has the potential to lead to imprecise results in cases where the inputs are variable. Rounding errors may ultimately lead to unintended calculations and even exploits of smart contract logic as a result.

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/fees/FeeManager.sol#L396

Tool used

Manual Review

Recommendation

The multiplication should be performed before the division. This can be achieved by adding parentheses to explicitly specify the order of operations. The corrected line of code should be: *_protocolShare = (protocolFee protocolFeeshareBps) / MAXBPS;**

ccashwell commented 4 months ago

This is literally just wrong. Division and multiplication have the same precedence, so the operators are evaluated left to right.