Dev Note #5: Specifying how to handle funds when pruning a vesting account

CosmWasm contracts use the BaseAccount type and ADR-028 module account addresses, ie. addresses in their own namespace. This ensure that no other keypair or module uses those addresses. An address collision is practically not possible since the sha256 algorithm is considered collision-resistant and we do not truncate the 32-byte output.

In Cosmos SDK however, with MsgCreateVestingAccount a feature was added long time ago that allows a non-owner of an address to set the account type of the address (to_address). This allows for address squatting, which is something we need to protect against. In CWA-2022-005 it is explained how address squatting can lead to Denial-of-service scenarios. The advisory also explains our mitigation strategy which aims to only accept BaseAccounts and prune other account types.

How to prune an account

In an ideal world, we could just call something like auth.deleteAccount(address) and delegate the logic of deleting an account to the SDK. However, such functionality does not exist. So we need to implement account pruning ourselves. Here we considered the following:

1. BaseAccount can be used for contracts. They don’t need to be pruned and their balance is preserved.
2. All types of vesting accounts are not supported as contract accounts. If we just deleted the vesting account, the bank balance would suddenly be unlocked.
3. Since the creation of vesting accounts on contract addresses is nothing that’s done in the regular use case of a blockchain, we need to assume that the vesting accounts are created and funded by an attacker.
4. Due to 2.+3. we decided to take away the funds by default, which can be done in multiple ways: (a) Burn, (b) Send to the community pool, or (c) Send to configured address.
5. Since a chain can have a large number of denoms, we only take away the original vesting denom and leave the rest untouched. This avoids a DoS attack by pruning becoming very expensive in terms of gas.
6. Account pruning should always succeed in order to avoid new DoS scenarios.

What’s the right™ way of taking away funds?

In Cosmos SDK we have supply tracking in the bank module as a feature for many years. This allows you to always read the available supply and both token issuance and burns are tracked. This burning approach is arguably superior to just sending the tokens to an account that has no keypair (such as 0x0 on Ethereum). For this reason, burning tokens has been chosen in the implementation that fixes CWA-2022–005.

It has come to our attention that some users consider burning inappropriate for their appchain and their tokenomics. What can be done here is to implement accountPruner in the wasm Keeper of wasmd with a custom implementation. This gives full flexibility to adapt the logic of the default VestingCoinBurner.

Feature request: Add token destination argument to NewVestingCoinBurner

In order for users to not worry about the full implementation of VestingCoinBurner we can make the token destination (point 4. from above) configurable in NewVestingCoinBurner. There an additional destination argument can be used to easily switch between 4.a, 4.b, 4.c.

If this functionality is still not flexible enough, accountPruner should be implemented differently, taking point 6 and CWA-2022-005 (address squatting) into account.

Changing the default

One reason for keeping the current behavior (burn tokens) by default is that it’s simple and clean, correctly keeping track of the supply. Another reason is that there are at least two other ways to burn tokens by an attacker, changing the total supply along the way:

1. Send tokens to a contract, which then uses BankMsg::Burn in CosmWasm. This is what the sink contract does here.
2. Use the new MsgBurn (Cosmos SDK 0.51+).

So for the time being we don’t see how a change of the default behaviour would be beneficial to the majority of users.

Conclusion

We are well aware that those multiple layers of mitigation strategies are confusing, hard to audit and reason about. But as long as this problem is not solved at its root (a non-owner being able to set an account type), we cannot avoid that.

We are happy to receive any feedback on the points discussed above in order to find the best solution for everyone.