Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure no "blocked" addresses are used where tokens are distributed #401

Open
alpe opened this issue Dec 14, 2021 · 2 comments
Open

Ensure no "blocked" addresses are used where tokens are distributed #401

alpe opened this issue Dec 14, 2021 · 2 comments
Labels
audit Something we should check "audit" before going to mainnet M Medium task, 1-3 days work
Milestone

Comments

@alpe
Copy link
Contributor

alpe commented Dec 14, 2021

Module accounts in the SDK are "blocked addresses". Any bank send TX would fail with them as token receiver.
We should ensure that contracts reject them as well.
As an example address (wasm module): tgrade1xds4f0m87ajl3a6az6s2enhxrd0wta48mx55a0

This is a placeholder issue to discuss and track. There is currently no query available, yet.
The query could go into wasmvm bank query or tgrade query. The later one is quicker to impl

@ethanfrey
Copy link
Contributor

There are two approaches:

  1. Check all possible failure modes and never call BankMsg::Send if it would fail.
  2. Handle errors (without aborting the process)

I favor 2 (using reply_on_error)

@ethanfrey ethanfrey added this to the v0.6.0 milestone Dec 14, 2021
@ethanfrey
Copy link
Contributor

This is not a contract functionality issue actually. It is two parts:

  1. x/wasm ensures any bank send to such an address with fail.
  2. Anywhere a failing bank send could break a contract, and the address to send to can be set by a user (without signing a tx with that account... not info.sender), we should use SubMsg::reply_on_error and handle a failure gracefully

I think (1) is done. (2) is more code audit level

@ethanfrey ethanfrey added the audit Something we should check "audit" before going to mainnet label Dec 27, 2021
@ethanfrey ethanfrey modified the milestones: v0.6.0, v1.0.0 Dec 27, 2021
@alpe alpe added the M Medium task, 1-3 days work label Jan 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit Something we should check "audit" before going to mainnet M Medium task, 1-3 days work
Projects
None yet
Development

No branches or pull requests

2 participants