code-423n4 / 2023-07-tapioca-findings

15 stars 10 forks source link

Usage of `BalancerStrategy.updateCache` will cause single sided Loss, discount to Depositor and to OverBorrow from Singularity #1447

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/balancer/BalancerStrategy.sol#L138-L147

Vulnerability details

Impact

The BalancerStrategy uses a cached value to determine it's balance in pool for which it takes Single Sided Exposure.

This means that the Strategy has some BPT tokens, but to price them, it's calling vault.queryExit which simulates withdrawing the LP in a single sided manner.

Due to the single sided exposure, it's trivial to perform a Swap, that will change the internal balances of the pool, as a way to cause the Strategy to discount it's tokens.

By the same process, we can send more ETH as a way to inflate the value of the Strategy, which will then be cached.

Since _currentBalance is a view-function, the YieldBox will accept these inflated values without a way to dispute them

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/balancer/BalancerStrategy.sol#L138-L147

    function _deposited(uint256 amount) internal override nonReentrant {
        uint256 queued = wrappedNative.balanceOf(address(this));
        if (queued > depositThreshold) {
            _vaultDeposit(queued);
            emit AmountDeposited(queued);
        }
        emit AmountQueued(amount);

        updateCache(); /// @audit this is updated too late (TODO PROOF)
    }

POC

Result

Imbalance Up -> Allows OverBorrowing and causes insolvency to the protocol Imbalance Down -> Liquidate Borrowers unfairly at a profit to the liquidator Sandwhiching the Imbalance can be used to extract value from the strategy and steal user deposits as well

Mitigation

Use fair reserve math, avoid single sided exposure (use the LP token as underlying, not one side of it)

Assessed type

ERC4626

c4-pre-sort commented 1 year ago

minhquanym marked the issue as primary issue

c4-sponsor commented 1 year ago

cryptotechmaker (sponsor) confirmed

c4-judge commented 1 year ago

dmvt marked the issue as selected for report