search

flow-usdc / flow-usdc

A FungibleToken-compatible fiat coin on the Flow blockchain, ERC20-alike with additional support for pausing and blocklisting.
MIT License
20 stars 15 forks source link

BUG: Functions to Create Important Admin Resources should not be public #56

Closed joshuahannan closed 3 years ago

joshuahannan commented 3 years ago

Many of the special admin resources that perform important actions for the usdc contract have creation function that are publicly accessible on the USDC contract: https://github.com/flow-usdc/flow-usdc/blob/main/contracts/FiatToken.cdc#L646-L664

Some of these resources, such as the minter, have other restrictions in place to prevent anyone from minting tokens without permission from a minter controller, but others, like the blocklister, don't have any restrictions, meaning that anyone could create a blocklister and blocklist as many arbitrary addresses as they want. It is important to not allow the public to create these resources even if there is a secondary line of defense because you want to reduce the attack surface of your contract as much as possible.

Suggested solution: Put these special resource creation function in a special admin resource and have the admin hand out these resources on a case by case basis.

whalelephant commented 3 years ago

Hi @joshuahannan,

Blocklister and Pauser require capabilities to be shared by the owner who owns the blocklistExecutor and pauseExecutor resources in order perform any execution https://github.com/flow-usdc/flow-usdc/blob/main/contracts/FiatToken.cdc#L600

We went with this and not to hand out resource directly as then the admin does not have the option to remove other account's stored resources.

This is our understanding, with this do you still suggest making those resource creating functions admin accessible only?

joshuahannan commented 3 years ago

actually that makes a lot of sense. Thanks for clarifying it for me!