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

15 stars 10 forks source link

SGLLeverage/BigBang `buyCollateral` Can Be Exploited to Steal Asset Approvals & Collateral #1086

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/singularity/SGLLeverage.sol#L147-L186 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L336-L375

Vulnerability details

BigBang and Singularity both have a buyCollateral function designed to allow users to lever up by borrowing more assets and buying collateral with it. In BigBang this is a public function, and in Singularity this is a accessible through a public function that ties into the SGLLeverage module. Effectively these have the same implementation.

Due to an ineffective check on the swap's final output collateralShare, an attacker can steal any user's approvals of asset to BigBang or Singularity. An attacker can also cause any user to borrow extra assets and buy collateral with it up until the point that the user is barely still solvent. Combined with a sandwich attack this can be used to steal the new collateral.

Notably the function takes a address from parameter, which allows the caller to specify any address as the user that is borrowing and buying collateral. Two checks are attempted to prevent this from being abused:

In actuality, the solvent(from) only limits the damage and the _allowedBorrow(from, collateralShare) check can be manipulated or outright bypassed.

Impact

The exploit hinges on abusing the _allowedBorrow(from, collateralShare) check. This is designed to check the allowance of msg.sender for from, but will pass in all cases if collateralShare is 0.

function _allowedBorrow(address from, uint share) internal {
    if (from != msg.sender) {
        if (allowanceBorrow[from][msg.sender] < share) {
            revert NotApproved(from, msg.sender);
        }
        allowanceBorrow[from][msg.sender] -= share;
    }
}

In order to get collateralShare as 0, the amountOut of the swapper.swap() call must be 0.

(amountOut, collateralShare) = swapper.swap(
    swapData,
    minAmountOut, //@audit minAmountOut attacker supplied
    from,
    dexData
);

The attacker provides minAmountOut, and in certain instances can manipulate the swap using a sandwich to return 0.

The protocol initially has 3 Swappers listed:

The UniswapV2Swapper and UniswapV3Swapper are both vulnerable to this attack, though with exceptions.

For UniswapV2Swapper, with a large enough flashmint the attacker can skew the curve enough to return 0. This would require reserves to be small and the flashmint to be inexpensive, but both are possible. BigBang supports USDO flashmints and has admin-setter functions for flashmint fees and max limits that both don't have bounds. Seems reasonable to think the flashmints have potential to be free and unlimited.

For UniswapV3Swapper, the same scenario as V2 applies as well as the possibility that full-range liquidity is not supplied. In this case the attacker needs less capital to skew the curve enough to return 0.

CurveSwapper is not vulnerable to this attack, as in the implementation it is not possible to return 0.

// CurveSwapper.sol
function _swapTokensForTokens(
    int128 i,
    int128 j,
    uint256 amountIn,
    uint256 amountOutMin
) private returns (uint256) {
    // .. snip
    curvePool.exchange(i, j, amountIn, amountOutMin);

    uint256 balanceAfter = IERC20(tokenOut).balanceOf(address(this));
    require(balanceAfter > balanceBefore, "swap failed");

    return balanceAfter - balanceBefore; //@audit always > 0
}

It seems reasonable to assume that in the future new swappers could be introduced - such as support for UniswapX. In a case of a new swapper implementation where a user is directly filled for minAmountOut, this vulnerability becomes trivial to exploit.

In a much more simple case than the amountOut == 0 extreme, if the attacker has any approvals from a user to borrow, that allowance can be abused. All of the current swappers are vulnerable to attacker sandwiches, which lowers amountOut. While having amountOut == 0 is difficult, skewing any of the swappers to have amountOut < allowanceBorrow[from][msg.sender] is much easier.

Stealing Approvals

Attacker controls supplyAmount and from. This portion of the code means any user's approvals of asset to BigBang or Singularity can be sent to the swapper.

uint256 supplyShare = yieldBox.toShare(assetId, supplyAmount, true);
if (supplyShare > 0) {
    yieldBox.transfer(from, address(swapper), assetId, supplyShare);
}

Stealing Collateral

Attacker controls borrowAmount and from. This portion of the code performs the _borrow action for the user up to borrowAmount.

uint256 borrowShare;
(, borrowShare) = _borrow(from, address(swapper), borrowAmount); //USDO shares

Note that the from user's solvency is checked at the end of the function via the solvent(from) modifier, so the attacker cannot cause a borrow that brings the user to insolvency.

Summary

In the maximum impact case, an attacker can steal any user's approvals of asset to BigBang or Singularity as well as force any user to borrow extra assets and buy collateral with it up until the point that the user is barely still solvent. Combined with a sandwich attack this can be used to steal the approved assets as well as the new collateral.

In the case that has reduced surface but easier replication, any user that has pre-approved another user for allowanceBorrow[from][msg.sender] falls victim to these same attacks.

Proof of Concept

The PoC hinges on the atomic sandwich of the swapper.

  1. Flashmint USDO or flashloan other asset to get USDO via another swapper.
  2. Sell USDO for tOFT in swapper to skew price
  3. call buyCollateral(target, max_borrowable, full_approvals, 0, swapper, dexData)
    1. this calldata will have the yieldbox.transfer line move all of the user's asset approvals to the swapper.
    2. this calldata will have the _borrow() line move extra borrowed assets to the swapper.
    3. swapper.swap() will fetch (0, 0) due to the skewed price
    4. _allowedBorrow() passes
    5. solvent(from) checks for the user's solvency, which passes
  4. Sell original tOFT into swapper for USDO at improved price
  5. Repay flashloan

Tools Used

Manual Review

Recommended Mitigation Steps

Perform transfer allowance checks for the user's approvals before performing the yieldBox.transfer(from, address(swapper), assetId, supplyShare);

Perform the _allowedBorrow check for the borrowShare`` that takes place beforeswapper.swap. ie:if (borrowShare > 0) { _allowedBorrow(from, borrowShare); }`

Assessed type

Other

c4-pre-sort commented 1 year ago

minhquanym marked the issue as duplicate of #147

c4-judge commented 1 year ago

dmvt changed the severity to 2 (Med Risk)

c4-judge commented 1 year ago

dmvt marked the issue as selected for report