Fixed: Vulnerable Accounts 2024-10-01

Summary of Impact

After the Crescendo upgrade it was reported that some contracts were updated to Cadence 1.0 in a manner that was not preserving functionality, which resulted in some public links getting migrated to capability controllers with additional permissions.

This left some objects in some accounts vulnerable. For example, some public links to some NFT collections, which previously only allowed deposits, got migrated to public capabilities which allowed withdrawals.

Most of the problematic contract updates were in NFTs. For example, previously a public link to an NFT collection would be exposed with a type that would only allow deposits.

Technical Summary of Issue

Before Cadence 1.0, limited access to stored objects was granted by calling the link function with a borrow type restricted to the NFT standard’s NonFungibleToken.CollectionPublic interface, and the NFT implementation’s “public collection” interface (e.g. ExampleNFT.ExampleNFTCollectionPublic), e.g. link<&{NonFungibleToken.CollectionPublic, ExampleNFT.ExampleNFTCollectionPublic}>(...). Both interfaces only made e.g. the deposit function publicly available.

In the Cadence 1.0 migration, links’ borrow types (reference types) were migrated from such restricted types to equivalent reference types with authorizations. The migration performed the rewrite this determining what functionality the Cadence 1.0 version of the reference type provided, and inferring the corresponding authorization (entitlements), by looking at the updated code of that particular type.

This mechanism worked correctly. However, some contracts, in particular some NFT contracts,
were updated to Cadence 1.0 in such a way that the migration inferred additional entitlements.
In particular, some NFT implementations updated their “public collection” interface to inherit from the full NonFungibleToken.Collection interface. This is safe when such a contract is deployed as a new contract. However, such an update was not safe in the Cadence 1.0 upgrade, because to the migration, the link appeared as fully authorized: If the link “had” access to all functionality in NonFungibleToken.Collection, which includes the withdraw function, the migration must ensure that it “keeps” the functionality, which required adding the Withdraw entitlement to the authorization of the migrated capability controller. This was obviously unlikely intended.

To clarify, the security issue was not in Flow or Cadence, but a result of unforeseen interactions between the migration code and otherwise benign inheritance chain changes in updated contracts.

Mitigation

To remedy the situation of leaving some NFT collections in some users accounts exposed, the Flow team:

  1. Implemented and ran a state migration on the execution states of the Flow Testnet and Mainnet networks which removed the authorizations of capability controllers and capabilities which were migrated from public links.
    Public links that were previously authorized or got migrated to authorized capability controllers were very likely unintentional.
  2. The behaviour of the Account.capabilities.publish, get, and borrow functions changed:
    It is no longer possible to publish, get, or borrow a public capability that is authorized.
    Publishing an entitled capability publicly is almost always a mistake.
    This change prevents accidentally exposing a stored value publicly.

Recognition

As core contributors to the Flow ecosystem, we take reported issues very seriously and would like to thank Austin Kline from Flowty (https://www.flowty.io) for reporting this issue responsibly through our Responsible Disclosure Policy.

1 Like