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

Combat "hammer" eviction attack #21460

Open
wjmelements opened this issue Aug 18, 2020 · 5 comments
Open

Combat "hammer" eviction attack #21460

wjmelements opened this issue Aug 18, 2020 · 5 comments

Comments

@wjmelements
Copy link
Contributor

It's currently fairly easy to evict most pending transactions from most of the network by issuing many high gas limit transactions from a handful of accounts and then cancel them all cheaply. The high gas limits ensure that few of the transactions will confirm before the cancel step. The cancel step replaces only the first unconfirmed transaction of each account with a transaction that drains the account, thereby evicting all subsequent transactions. Those subsequent transactions can thus evict a large number of competitor transactions quickly and cheaply. It is believed that a similar strategy was used against makerdao collateral on Black Thursday.

@holiman
Copy link
Contributor

holiman commented Sep 22, 2020

Any solution to this problem would either make replacement txs impossible or very expensive. So it's basically UX versus security. If we make replacements very expensive (say, replace only accepted if it's worth more than 100% of the original one), then a user who submits a tx with wrong (very high) gasprice is screwed.

I guess we may have to revise the limits at some point, to prevent these types of shenanigans.

@wjmelements
Copy link
Contributor Author

Could we reduce the limit on pending transactions per-account to make this more expensive?

@karalabe
Copy link
Member

  • Option 1 is to disable replacement transactions altogether, but IMHO that is a bit nasty because it's a nice UX to be able to override an old transactions if for example your gas price is too low.
  • Option 2 is to only allow repricing, but not changing the value. This would break support for cancellations where you change the entire tx to something else.
  • Option 3 is to make replacements more expensive, maybe drastically so. This imho would go to the detriment of normal users, but won't much prevent attacks, you just change the amount of gains the attacker needs before it's worth triggering it.

My proposal is option 4, whereby a transaction replacement also takes into consideration the subsequent transactions getting invalidated. (Important to note, the only way to invalidate a tx is to run out of balance.). What we can do, is to check if a replacement tx lowers the remaining balance compared to the original transaction, and if it does, whether the balance gets lowered enough to kick out subsequent txs. If yes, the replacement should be rejected.

This proposal does not interfere with normal users using Ethereum, but it ensures that once a tx with a certain nonce enters the pool it stays there.


Yes, it could still be kicked out using another account, but that should outbid the original ones. We could add a new rule to txpool so that higher fee txs from other accounts would need to be 10% more expensive to kick out old txs already pooled... but this might backfire with fee spikes, so not sure if it's a good idea.

@fjl
Copy link
Contributor

fjl commented Nov 26, 2020

Here's some more info about the proposal @karalabe outlined above. I'm adding this because I got confused initially and didn't understand how it could work.

In Ethereum, all TXs must originate in an EOA and contracts can never 'take value' out of an EOA. This means the balance of an EOA can never decrease unless it sends a TX, i.e. if it has balance V at the head block, and we have some TXs Ta, Tb, Tc spending value Va, Vb, Vc, we know that the remaining balance will be at least V-Va-Vb-Vc. It might be higher (due to incoming transfers), but that doesn't really matter because we're looking for a worst-case prediction of balance here.

To implement this proposal, we'd basically just start tracking the 'expected minimum balance' of all accounts in the pool, just like we already track the 'expected nonce' in the txNoncer. Then, when inserting transactions, we could easily check whether any TX replacement will lower the expected balance and potentially reject the replacement as @karalabe wrote.

@fjl
Copy link
Contributor

fjl commented Nov 26, 2020

The benefit of this scheme is that cancelling TXs will still work for most people, as long as they're not piling up future transactions. This is very good. Thinking about it more, this 'expected minimum balance' is also generally useful for deciding whether TXs should be pooled. Right now, it's actually possible to pile up TXs where all but the first one spend non-existing balance. This change would allow us to prevent pooling of such transactions.

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

No branches or pull requests

6 participants