Re-entrancy protection does not work for certain curve pools

Github username: @ArnieGod Submission hash (on-chain): 0xf03db129703b98f16b348137ff7093b08f2f6fdb415e4566232949e3e5b3898b Severity: medium severity

Description:

Vulnerability

Summary

Vulnerability Detail

curve is subject to read only reentrancy and the protocol is well aware of such attack vector so the protocol implements reentrancy protection. The protocols re entrancy protection is adequate for curve pools with up to 3 tokens. However the protocol does not offer this same protection for curve pools of 4 tokens.

    /**
     * @dev Helper to prevent read-only re-entrancy attacks with virtual price. Only needed if the underlying has ETH.
     * @param curve_pool The curve pool address (not the token address!)
     **/
    function check_reentrancy(address curve_pool, uint256 num_tokens) internal {
        //makerdao uses remove_liquidity to trigger reentrancy lock
        // if(try_admin_fees(curve_pool)){
        //     return;
        // }
        // address owner = ICurvePool(curve_pool).owner();
        // if(Address.isContract(owner)) {
        //     if(try_admin_fees(owner)){
        //         return;
        //     }
        // }
        if(try_remove_liquidity_one_coin(curve_pool)){
            return;
        }
        if(num_tokens==2 && try_remove_liquidity_2(curve_pool)){
            return;
        }
        if(num_tokens==3 && try_remove_liquidity_3(curve_pool)){
            return;
        }
        uint[3] memory amounts = [uint(0), uint(0), uint(0)];
        ICurvePool2(curve_pool).remove_liquidity(0, amounts);
        revert(Errors.VO_REENTRANCY_GUARD_FAIL);
    }

first the code tries to call remove liquidity on one coin if the remove liquidity on the coin does not work, the code try to call

remove_liquidity with uint[2] memory amounts = [uint(0), uint(0)];

then if such call does work this is called

remove_liquidity with uint[3] memory amounts = [uint(0), uint(0)];
however, there are curve pool that support 4 tokens together

https://github.com/curvefi/curve-contract/tree/master/contracts/pools https://curve.fi/#/ethereum/pools/y/deposit/ see this https://github.com/curvefi/curve-contract/tree/master/contracts/pools/y the onchain contract is linked below https://etherscan.io/address/0x45f783cce6b7ff23b2ab2d70e416cdb7d6055f51#writeContract if we see the write function 4. remove_liquidity (a file with the image is linked below) there is no remove liqudity for one coin and when remove liquidity is used, an array of length 4 is required. this means the ready only reentrancy protection does not work for the curve pool with 4 token and does not support curve pool with 4 token.

Impact

Re entrancy guard does not work for curve pools with 4 tokens, additionally the protocol does not support curve pools with 4 tokens

Code Snippet

https://github.com/hats-finance/VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f/blob/fb396a3fa412e643de7d8a1fd8a0268ab3a2f993/packages/contracts/contracts/protocol/oracles/CurveOracle.sol#L67-L90

Recommendation

this snippet of code can be used to add support for 4 token curve pools

 if(num_tokens==4 && try_remove_liquidity_4(curve_pool)){
            return;
 }

hats-finance / VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f