Do not check allowance before token transfer

Summary

Do not check allowance before token transfer.

Vulnerability Detail

The vault does the asset.approve(address(collateral)) only in the initialization. The vault calls the collateral.depositTo() and the depositTo() executes the token transfer. Once the allowance is not enough, the _updateCollateral() will fail to function properly.

    function _updateCollateral(IProduct product, UFixed18 targetCollateral) private {
        ...
        if (currentCollateral.lt(targetCollateral))
            collateral.depositTo(address(this), product, targetCollateral.sub(currentCollateral));
    }

This is the code snippet of depositTo():

    function depositTo(address account, IProduct product, UFixed18 amount)
    external
    nonReentrant
    notPaused
    notZeroAddress(account)
    isProduct(product)
    collateralInvariant(account, product)
    {
        _products[product].creditAccount(account, amount);
        token.pull(msg.sender, amount);
        ...
    }

The token.pull() will do the token transfer, and it will revert once the allowance is not enough.

Impact

The _updateCollateral() will not work when the allowance is not enough.

Code Snippet

https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial-vaults/contracts/balanced/BalancedVault.sol#L532

Tool used

Manual Review

Recommendation

Check the allowance is enough.

sherlock-audit / 2023-05-perennial-judging

ashirleyshe - Do not check allowance before token transfer #230