sherlock-audit / 2023-07-blueberry-judging

2 stars 1 forks source link

Hama - Precision Loss in Calculation of lessShare Variable #67

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

Hama

medium

Precision Loss in Calculation of lessShare Variable

Summary

The code snippet provided contains a potential vulnerability related to precision loss during the calculation of the lessShare variable. This vulnerability arises from the order of operations in the calculation, which involves multiplication and division operations. As a result, there is a risk of losing decimal precision in the calculated value, leading to inaccurate results in certain scenarios.

Vulnerability Detail

In the code snippet, the lessShare variable is calculated as follows:

uint256 lessShare = paid == oldDebt  ? oldShare   : (paid * totalShare) / totalDebt;

This can potentially cause precision loss when paid * totalShare is lower and almost equal to totalDebt. And as such 0 would be subsracted from bank.totalShare and pos.debtShare.

Impact

The precision loss in the calculation of lessShare can lead to inaccurate results, particularly when dealing with large values. This imprecision could have negative effects on the system's accounting and bookkeeping, potentially affecting user balances and overall financial integrity.

Code Snippet

https://github.com/sherlock-audit/2023-07-blueberry/blob/main/blueberry-core/contracts/BlueBerryBank.sol#L833

function _repay(
    uint256 positionId,
    address token,
    uint256 amountCall
) internal returns (uint256, uint256) {
    ..
    if (paid > oldDebt) revert Errors.REPAY_EXCEEDS_DEBT(paid, oldDebt); /// prevent share overflow attack
    uint256 lessShare = paid == oldDebt
        ? oldShare
        : (paid * totalShare) / totalDebt;
    bank.totalShare -= lessShare;
    pos.debtShare -= lessShare;
    return (paid, lessShare);
}

Tool used

Manual Review

Recommendation

Perform the necessary calculations using the decimal values to avoid precision loss. This calculation should now retain better precision compared to using regular integers.

sherlock-admin2 commented 1 year ago

3 comment(s) were left on this issue during the judging contest.

0xyPhilic commented:

invalid because the chance of this issue occuring is extremelly low

darkart commented:

Invalid

Kral01 commented:

the likelihood is almost equal to none