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

ERC 20 Bridge Withdraws to Ethereum: Improving Security / Safety #293

Open
drinkcoffee opened this issue Jul 11, 2023 · 3 comments
Open

Comments

@drinkcoffee
Copy link
Contributor

ERC 20 Bridges / withdraws to Ethereum: To withdraw a token to Ethereum, the withdrawal contract on Ethereum is RootERC20Predicate. At present, this contract has the following features:

  • Any amount of any linked token can be withdrawn. The withdrawal is triggered on L2, with the exit being able to be executed as soon as the Checkpoint is submitted.
  • There is no pause capability on withdraws.

I suggest the creation of an extension to RootERC20Predicate that does the following:

  • Having a pause capability that could pause calls to _withdraw() https://github.com/0xPolygon/core-contracts/blob/main/contracts/root/RootERC20Predicate.sol#L127
  • For any withdraw above a selectable limit (by an admin), the withdraw sits in the contract for 24 hours (again programmable) before being released. This would complicate the user interface for people creating the UI for the bridge.
  • Have a rate limit "alarm" that would detect large outflows in a given time period. The "alarm" could emit an event, which would be easy for catch and surface in a dashboard. We could also consider when the alarm occurs, allowing the contract itself to automatically pause the withdraw function, or have all withdraws go through a 24 hour hold.

The goal of all of the suggestions is to have a way for us to slow down / stop an in progress attack.

Maybe the new contract could be called RootERC20PredicateLimits.

I am happy to commence work on this. However, before I start, I would like to have a discussion to ensure we have alignment on how this could be added to the repo.

@sambacha
Copy link

ERC 20 Bridges / withdraws to Ethereum: To withdraw a token to Ethereum, the withdrawal contract on Ethereum is RootERC20Predicate. At present, this contract has the following features:

  • Any amount of any linked token can be withdrawn. The withdrawal is triggered on L2, with the exit being able to be executed as soon as the Checkpoint is submitted.
  • There is no pause capability on withdraws.

I suggest the creation of an extension to RootERC20Predicate that does the following:

  • Having a pause capability that could pause calls to _withdraw() https://github.com/0xPolygon/core-contracts/blob/main/contracts/root/RootERC20Predicate.sol#L127
  • For any withdraw above a selectable limit (by an admin), the withdraw sits in the contract for 24 hours (again programmable) before being released. This would complicate the user interface for people creating the UI for the bridge.
  • Have a rate limit "alarm" that would detect large outflows in a given time period. The "alarm" could emit an event, which would be easy for catch and surface in a dashboard. We could also consider when the alarm occurs, allowing the contract itself to automatically pause the withdraw function, or have all withdraws go through a 24 hour hold.

The goal of all of the suggestions is to have a way for us to slow down / stop an in progress attack.

Maybe the new contract could be called RootERC20PredicateLimits.

I am happy to commence work on this. However, before I start, I would like to have a discussion to ensure we have alignment on how this could be added to the repo.

The alternative solution would be a fee on transfer mechanism in which transfers exceeding a calculated value / constant value (eg % of total floating supply) would incur a “surcharge”.

the issue with that approach is composability from an integrator viewpoint

@drinkcoffee
Copy link
Contributor Author

The alternative solution would be a fee on transfer mechanism in which transfers
exceeding a calculated value / constant value (eg % of total floating supply) would
incur a “surcharge”

Incurring a surcharge wouldn't stop the attack. The attack could still proceed.

@drinkcoffee
Copy link
Contributor Author

I have implemented the changes in a fork of this repo and written a threat model in preparation for external contract audit. The threat model explains my thinking more clearly.
https://github.com/immutable/poly-core-contracts/blob/main/audits/202308-threat-model-erc20-bridge.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants