sherlock-audit / 2023-07-blueberry-judging

2 stars 1 forks source link

twcctop - AuraSpell#_getJoinPoolParamsAndApprove Leads to maxAmountsIn[i] Mismatch with Existing LP Tokens #77

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

twcctop

high

AuraSpell#_getJoinPoolParamsAndApprove Leads to maxAmountsIn[i] Mismatch with Existing LP Tokens

Summary

The maxAmountsIn[i] mismatch occurs when an LP token exists. This mismatch arises due to a discrepancy in the increase logic between amountsIn and maxAmountsIn arrays. This vulnerability is present in the AuraSpell#_getJoinPoolParamsAndApprove function.

Vulnerability Detail

In the AuraSpell#_getJoinPoolParamsAndApprove function:

for (i; i != length; ) {
    if (tokens[i] != lpToken) {
        amountsIn[j] = IERC20(tokens[i]).balanceOf(address(this));
        if (amountsIn[j] > 0) {
            _ensureApprove(tokens[i], vault, amountsIn[j]);
        }
        ++j;
    } else isLPIncluded = true;

    maxAmountsIn[i] = IERC20(tokens[i]).balanceOf(address(this));

    unchecked {
        ++i;
    }
}

if (isLPIncluded) {
    assembly {
        mstore(amountsIn, sub(mload(amountsIn), 1))
    }
}

When tokens[i] is equal to lpToken, isLPIncluded is set to true, and the ++j increment is skipped. This issue causes a discrepancy between amountsIn and maxAmountsIn if the LP token is not positioned at the last index.

Impact

If isLPIncluded is involved in the openPositionFarm process of adding liquidity, it may lead to a revert and potentially result in a denial-of-service attack on the system.

Code Snippet

https://github.com/sherlock-audit/2023-07-blueberry/blob/main/blueberry-core/contracts/spell/AuraSpell.sol#L309-L323

Tool used

Manual Review

Recommendation

To address this issue:

Ensure that the amountsIn and maxAmountsIn arrays have the same length and that their data matches. This can be done by making sure that the incrementing logic for both arrays is consistent, even when an LP token is encountered. This will prevent the mismatch between the two arrays and potential vulnerabilities.

Duplicate of #125

sherlock-admin2 commented 1 year ago

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

Kral01 commented:

the scope of j is used only inside the if{}