sherlock-audit / 2023-04-blueberry-judging

8 stars 5 forks source link

0x52 - Balance check for swapToken in ShortLongSpell#_deposit is incorrect and will result in nonfunctional contract #133

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

0x52

medium

Balance check for swapToken in ShortLongSpell#_deposit is incorrect and will result in nonfunctional contract

Summary

The balance checks on ShortLongSpell#_withdraw are incorrect and will make contract basically nonfunctional

Vulnerability Detail

swapToken is always vault.uToken. borrowToken is always required to be vault.uToken which means that swapToken == borrowToken. This means that the token borrowed is always required to be swapped.

ShortLongSpell.sol#L83-L89

    uint256 strTokenAmt = _doBorrow(param.borrowToken, param.borrowAmount);

    // 3. Swap borrowed token to strategy token
    IERC20Upgradeable swapToken = ISoftVault(strategy.vault).uToken();
    // swapData.fromAmount = strTokenAmt;
    PSwapLib.megaSwap(augustusSwapper, tokenTransferProxy, swapData);
    strTokenAmt = swapToken.balanceOf(address(this)) - strTokenAmt; <- @audit-issue will always revert on swap

Because swapToken == borrowToken if there is ever a swap then the swapToken balance will decrease. This causes L89 to always revert when a swap happens, making the contract completely non-functional

Impact

ShortLongSpell is nonfunctional

Code Snippet

ShortLongSpell.sol#L160-L202

Tool used

Manual Review

Recommendation

Remove check

Gornutz commented 1 year ago

https://github.com/Blueberryfi/blueberry-core/commit/482ccbd40044ba30dde0f028d968ae4e9ae9e262

IAm0x52 commented 1 year ago

Fix looks good. Contract now correctly tracks the balances of the destination token rather than the token being swapped.