sherlock-audit / 2022-09-notional-judging

4 stars 2 forks source link

ctf_sec - Deployments.BALANCER_VAULT.swap uint256 amonutsOut return value not handled in BalancerUtils.sol #116

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ctf_sec

medium

Deployments.BALANCER_VAULT.swap uint256 amonutsOut return value not handled in BalancerUtils.sol

Summary

Maturity validation check is missing in VaultAccount.sol if fCashToBorrow is larger than 0

Vulnerability Detail

currently, in BalanerUtils.sol the _swapGiveIn just use the before and after token balance to check the token received.

        amountOut = IERC20(tokenOut).balanceOf(address(this));
        Deployments.BALANCER_VAULT.swap({
            singleSwap: IBalancerVault.SingleSwap({
                poolId: poolId,
                kind: IBalancerVault.SwapKind.GIVEN_IN,
                assetIn: IAsset(tokenIn),
                assetOut: IAsset(tokenOut),
                amount: amountIn,
                userData: new bytes(0)
            }),
            funds: IBalancerVault.FundManagement({
                sender: address(this),
                fromInternalBalance: false,
                recipient: payable(address(this)),
                toInternalBalance: false
            }),
            limit: limit,
            deadline: block.timestamp
        });
        amountOut = IERC20(tokenOut).balanceOf(address(this)) - amountOut;

however, the function Deployments.BALANCER_VAULT.swap returns amountsOut, the function does not properly handle the return value

According to the code of the balancer Pool Vault Implementation.

https://github.com/balancer-labs/balancer-v2-monorepo/blob/2c4d727bb30d66392bab7aae1019f389fe2d03a6/pkg/vault/contracts/Swaps.sol#L70

   whenNotPaused
   authenticateFor(funds.sender)
   returns (uint256 amountCalculated)

the amountCalculated can be used with the before and after balance together to verify the token amount received is valid

Impact

Without handling the return value of the function swap, it is possible that we receive less than expected amount of token.

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/pool/BalancerUtils.sol#L125-L153

Tool used

Manual Review

Recommendation

We recommend the project handle the return value from the swap and use the returned amountsOut with the before and after balance to validate the transactions.

For example, if the amoutnsOut is 100, the difference between the before and after balance is 50, we know something is wrong and transaction should be reverted.