Secure BankKeeper with Dynamic Capabilities

[AdityaSripal]

Summary

Creating a module account should return a dynamic capability. Modules must pass in the dynamic capability in order to authenticate and spend from their module account.

Problem Definition

Currently any module with access to the supplykeeper can spend from any module account so long as they know the name of the module account. Thus, a malicious module can spend funds out of a module account it does not know so long as it knows the name of the other module account. This is usually trivial since module account names are predictable and queryable.

In the user account model, we prevent users from spending funds from unowned accounts whose address they know by requiring a cryptographic signature to authenticate expenditures.

Similarly in the module account model, we can prevent modules from spending funds from an unowned module account by requiring they pass in a dynamic capability bound to the module account to authenticate expenditures

Proposal

SupplyKeeper is passed in a ScopedCapabilityKeeper. Upon creation of a module account, it returns a new Capability which is associated with that module account name.

The module that creates the account can then claim this capability.

SpendCoinsFromModuleToAccount now takes in a capability. supply.Keeper authenticates this capability using the name of the module account. If authentication succeeds, allow the spend. If it fails, return an error

All modules that need to create and spend from module accounts must now have their own ScopedCapabilityKeeper so they can claim and retrieve capabilities.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[AdityaSripal]

cc: @alexanderbez

Think this is probably blocked on getting IBC code into master. I can work on this after that is done

[alexanderbez]

Neat! We can certainly cherry-pick the x/capability module into master without having to wait on IBC. It should be extremely straight forward. Also, we planned on merging the supply module into the bank module. I doubt this would cause any conflicts with the design proposal. Please correct me if I'm wrong.

[AdityaSripal]

Great! Do you want to cherry-pick the commit, and then I can work on this issue? No, merging supply into bank won't be an issue

[alexanderbez]

Yeah, I think that'd be a good approach. That way it's not blocking and we get this proposal in sooner rather than later.

[fedekunze]

can we close this issue now?

[AdityaSripal]

No it hasn't been implemented yet

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[tac0turtle]

closing this in favour of bank v2 adr