Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Remove All Assets and Uniques from NonTransfer Proxy #486

Merged
3 commits merged into from
Jun 16, 2021

Conversation

shawntabrizi
Copy link
Member

@shawntabrizi shawntabrizi commented Jun 10, 2021

Given the nature of the Assets and Uniques pallet, it seems very sensible to disable all access to this pallets from the NonTransfer proxy.

Combinations of calls like mint and burn can be used to emulate transfer behaviors, and who knows what new functions will be added.

Given that we have explicit Proxies for AssetOwner and AssetManager, which specifically handle management of these pallets, I see little reason for NonTransfer proxy to have this risk.

Fixes: https://github.com/paritytech/srlabs_findings/issues/91

@xlc
Copy link
Contributor

xlc commented Jun 10, 2021

Not really directly related but I am thinking if it make sense to introduce an event based proxy filtering mechanism. i.e. fail the transaction if a specific event is emitted.

@shawntabrizi
Copy link
Member Author

@xlc would only work for transactional extrinsics?

@xlc
Copy link
Contributor

xlc commented Jun 10, 2021

Right. So that't depends on making everything transactional first.
Another way could be annotate each calls with tags and filter based on whitelist / blacklist of the tags. I guess this won't happen though... But the current proxy filter is really error prone.

Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unique is not in statemint and westmint?

@shawntabrizi
Copy link
Member Author

shawntabrizi commented Jun 13, 2021

@bkchr nope. should it be?

EDIT: #493

@shawntabrizi
Copy link
Member Author

bot merge

@ghost
Copy link

ghost commented Jun 16, 2021

Trying merge.

@ghost ghost merged commit 7d6f46b into master Jun 16, 2021
@ghost ghost deleted the shawntabrizi-proxy-filter-update branch June 16, 2021 23:02
chevdor pushed a commit to chevdor/cumulus that referenced this pull request Jun 24, 2021
* remove all assets and uniques from `NonTransfer` proxy

* fix merge
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants