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

How to prevent a Call to be dispatched indirectly #4915

Closed
xlc opened this issue Feb 13, 2020 · 2 comments · Fixed by #6131
Closed

How to prevent a Call to be dispatched indirectly #4915

xlc opened this issue Feb 13, 2020 · 2 comments · Fixed by #6131
Labels
Z7-question Issue is a question. Closer should answer.

Comments

@xlc
Copy link
Contributor

xlc commented Feb 13, 2020

Calls can be dispatch indirectly by for example utility.as_sub. But for indirect dispatched calls, our SignedExtension will not be able to detect those calls and perform additional validations before there enters tx pools.

For example, in our oracle module, only whitelisted users are allowed to submit oracle.feed_value and the tx fee is configured to be zero. We also want to ensure each whitelisted user can only submit up to one oracle.feed_value per block (so they cannot DOS the network permanently).

Because those tx are free, we need to ensure invalid txs are rejected which can be enforced by our CheckOperator signed extension.

But in our implementation, we are only able to handle calls that made to oracle module and validate them. If attacker use utility.as_sub to dispatch the call indirectly, the validate logic will be bypassed completely.

It is possible to modify our implementation to look for Calls from utility pallet, but it just does not scales and not a good solution.

The quick fix to prevent this issue is to be able to detect if a dispatchable is dispatched indirectly, and panic if it is, which will prevent the trouble tx to be added to tx pool.

On the other hand, many dispatchables are designed to be dispatched indirectly only (e.g. system.set_code). So we may want a better design to handle those cases:

  • Calls that can be dispatched directly or indirectly (i.e. the current behavior)
  • Calls that can only be dispatched indirectly (e.g. system.set_code)
  • Calls that can only be dispatched directly to avoid bypass additional validation logic.

The work on our side: open-web3-stack/open-runtime-module-library#86

@bkchr
Copy link
Member

bkchr commented Feb 13, 2020

While you can use a SignedExtension for early rejection of invalid transactions. You should not use a SignedExtension to check the origin of a transaction. Not sure why you not check the origin inside of the function? What is the problem?

@bkchr bkchr added the Z7-question Issue is a question. Closer should answer. label Feb 13, 2020
@xlc
Copy link
Contributor Author

xlc commented Feb 13, 2020

Please let me clarify this.

I am presenting one issue:

It is possible to bypass validation logic in validate by dispatch the call indirectly (e.g. utility.as_sub).

For example, contracts pallet checks the transaction cannot exceed block gas limit in validate. The check won't be performed if the call is dispatched by utility.as_sub.

Call::put_code(gas_limit, _)

This basically prevents anyone to enforce security related logics in validate, which will be required if I want to reject certain tx from been included into block / inserted into tx pool.

If fact, I view this could cause potential security issues because bypassable validation logic just doesn't make sense. A useful but insecure tool is dangerous.

A kind of related issue is charge tx fee is performed in validate, which is bypassed when they are dispatched indirectly. #3921

Currently, validate is the only place for runtime developers to customize the behavior of tx pool, to allow rejecting bad txs or adjust priority. With this issue, developers cannot rely on it anymore.

utility.as_sub forwards dispatch weight but does not forward validation logic.


To workaround this issue, I want a way to detect if a oracle call is dispatched indirectly and reject / slash it. This ensures the call that bypass validate logic are invalid / slashable.


Here are some background contexts of why I am seeing this issue is a blocker for us.

We are building an oracle module, and here are some high level goals: open-web3-stack/open-runtime-module-library#80

We want oracle operators be able to submit feed value tx without charging them fee (FreeOeprational). But of course this could be a DOS vector that needs some careful considerations.

So my solution is:

  • only allow whitelisted operator to be able to submit oracle feed value tx
  • bad oracle operators can be slashed and removed from whitelist
  • each oracle operator can only submit one oracle feed value tx per block
    • this means they cannot DOS the network by filling block with max priority operational txs
  • oracle tx are max priority operational to reduce the possibility of front-running
    • related: A way to enforce transaction order in a block #4177
    • tx with max priority will be included in the beginning of block, which prevents front running
    • operational class will ensure attacker cannot prevent inclusion of the tx by filling the block with high tip txs (assuming block producers are not controlled by attacker directly)
  • we implement the ensure origin and other check logics in validate to enforce above rules
    • invalid oracle tx will be rejected straightaway by tx pool
    • malicious validator cannot craft blocks that including invalid oracle tx
    • this relies that validate cannot be bypassed

Please provide feedbacks if you can see a better ways to achieve our goal.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Z7-question Issue is a question. Closer should answer.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants