sherlock-audit / 2023-05-Index-judging

6 stars 3 forks source link

Phantasmagoria - addLiquidity() function of ammModule.sol reverts every time when adding liquidity #292

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Phantasmagoria

medium

addLiquidity() function of ammModule.sol reverts every time when adding liquidity

Summary

addLiquidity() function of ammModule.sol reverts every time when adding liquidity due to underflow

Vulnerability Detail

_executeAddLiquidity function is a part of the addLiquidity function that is responsible for transferring of components to a specified AMM pool. After liquidity was added _updateComponentPositions is called:

function _updateComponentPositions(ActionInfo memory _actionInfo) internal returns(int256[] memory) {
        int256[] memory componentsReceived = new int256[](_actionInfo.components.length);

        for (uint256 i = 0; i < _actionInfo.components.length; i++) {

            (uint256 currentComponentBalance,,) = _actionInfo.setToken.calculateAndEditDefaultPosition(
                _actionInfo.components[i],
                _actionInfo.totalSupply,
                _actionInfo.preActionComponentBalances[i]
            );
            // @audit
            componentsReceived[i] = currentComponentBalance.toInt256()
                                        .sub(_actionInfo.preActionComponentBalances[i].toInt256());
        }

    return componentsReceived;
    }

In the function _updateComponentPositions we subtract the value of currentComponentBalance from preActionComponentBalances to obtain componentReceived. The value of currentComponentBalance is obtained from the calculateAndEditDefaultPosition function, and it represents the balance of the component after liquidity has been added and tokens have been transferred to the pool:

uint256 currentBalance = IERC20(_component).balanceOf(address(_setToken));

An issue arises here because preActionComponentBalances consistently appears to be greater than currentComponentBalance As a result, subtracting these two values leads to a revert of the transaction, making it impossible to add liquidity.

Impact

Adding liquidity is impossible due to underflow

Code Snippet

https://github.com/IndexCoop/index-protocol/blob/86be7ee76d9a7e4f7e93acfc533216ebef791c89/contracts/protocol/modules/v1/AmmModule.sol#L131-L135

https://github.com/IndexCoop/index-protocol/blob/86be7ee76d9a7e4f7e93acfc533216ebef791c89/contracts/protocol/modules/v1/AmmModule.sol#L533-L535

Tool used

Manual Review

Recommendation

You can add parameter that will be true when adding liquidity and false when removing

function addLiquidity(
        ISetToken _setToken,
        string memory _ammName,
        address _ammPool,
        uint256 _minPoolTokenPositionUnit,
        address[] calldata _components,
        uint256[] calldata _maxComponentUnits
        bool isAdd
)

And in _updateComponentPositions this parameter should be checked like this:

if(isAdd) {
        componentsReceived[i] = _actionInfo.preActionComponentBalances[i].toInt256().sub(currentComponentBalance.toInt256());
} else {
        componentsReceived[i] = currentComponentBalance.toInt256().sub(_actionInfo.preActionComponentBalances[i].toInt256());
}