Description

Domain tokens can be permanently lost due to interactions between token transfer and the bridge mechanisms.

The specific scenario occurs through the following sequence:

1. Alice owns a non-EOI ownership token for alice.xyz on chain A.

2. Alice transfers the token to Bob, triggering the _beforeTokenTransfer hook in NameToken. This removes ownership of the token, setting the name.claimedBy address to the Doma Proxy address.

3. The hook makes a cross-chain call to DomaRecordProxyFacet::tokenTransfer, which sets name.claimedBy to the Doma Proxy address.

4. Bob attempts to bridge the token to chain B by calling ProxyDomaRecord::bridge. Particularly, he does this before claiming ownership of the token.

5. The token is burned on chain A as part of the bridge process.

6. Then, DomaRecordProxyFacet::bridge is called on the Doma chain.

7. The function checks LibDoma.isClaimedByDomaProxy(name.claimedBy) and returns early since it is true, as the name has not been claimed by anyone yet.

8. No token is minted on chain B, leaving Bob with no token while the domain state remains in the DomaRecord contract.

The result is that the token completely disappears while the system still believes the domain is tokenized, making it permanently impossible to create new tokens for that domain.

Note that this can also occur if the bridging process is interrupted due to any reason (such as, for example, a cross-chain function call reverting), as the token will always be burned on the source chain first, and any interruptions will lead to the token never being minted on any other chains.

Impact

This issue can cause permanent loss of tokenization functionality for affected domains.

Proof of Concept

To demonstrate this issue, we chose to modify the early return statement in DomaRecordProxyFacet::bridge() to a revert(), like so:

function bridge(
    /* ... */
) external onlyCrossChainSender {
    // [ ... ]

    // If it's an ownership token, we might need to update claimedBy status to a new owner
    if (nameToken.ownership) {
        // If it's in DOMA proxy, skip update
        if (LibDoma.isClaimedByDomaProxy(name.claimedBy)) {
            revert(); // <==== Zellic - modify this line
        }

        name.claimedBy = CAIP.format(targetChainId, targetOwnerAddress);
    }

    // [ ... ]
}

Then, we added the following test to test/doma-record/DomaRecordProxyFacet.test.ts and ran it with npx hardhat test --grep audit:

describe('audit', () => {
  it('audit - bridging does not complete when the token is unowned', async () => {
    await utils.addRegistrar();
    await utils.createName();
    await domaRecordFacet.connect(owner).setProxyContract(DOMA_CHAIN_ID, PROXY_CONTRACT_ADDRESS);
    await utils.transferOwnershipTokenToAnotherUser();

    const ownershipTokenId = generateTestOwnershipTokenId();

    // This token is does not have it's ownership claimed. Bridge it
    const tx =  domaRecordProxyFacet
      .connect(crossChainSender)
      .bridge(ownershipTokenId.toString(), DOMA_CHAIN_ID, user.address, TEST_CORRELATION_ID);

    // The revert simulates the early return. The code expects no revert (i.e the bridging
    // completes all the way). However, this will revert and prevent the bridging process from
    // completing.
    await expect(tx).to.not.be.reverted;
  });
})

Running the test shows that the bridging process ends early, and thus leads to no tokens being minted on the remote chain.

Recommendations

We recommend not allowing users to perform any critical actions using their name tokens without claiming ownership of the token first. This can be done using a modifier on most external functions on the NameToken and OwnershipToken contracts.

Additionally, we also recommend being very vigilant about ensuring that the bridging process will either always succeed completely or not succeed at all. Partial bridging will end up with unintended consequences, such as the loss of name tokens.

Remediation

This issue has been acknowledged by D3, and a fix was implemented in commit 3dfb0670↗.