How does 'forbid-evm-reentrancy' work?

[maksym-nyrka]

Hi, in our blockchain we've added EVM support with pallet-evm.
I wonder how does 'forbid-evm-reentrancy' feature actually work? It's not so obvious for me from PR.
I tried to reproduce this example of re-entrancy and attack was successful.

[arturgontijo]

Hey @maksym-nyrka, thanks for the report!

Do you have an already setup repo so we can easily reproduce that?

I see that your node is using the Astar's Frontier implementation, it would be better to use this repo instead of https://github.com/AstarNetwork/frontier too.

[akorchyn]

It will create dependency hell that is hard to resolve as other pallets have astar-requirements. We don't have a minimum reproducible example, but you can try to run cargo run --release -- -dev, but it's not that small.

First, just to clarify, is this feature preventing reentrancy smart contract attack?

[arturgontijo]

First, just to clarify, is this feature preventing reentrancy smart contract attack?

If you mean, does frontier prevent the InsecureEtherVault (from this example) from be drained? No it does not.

That smart contract allows it, I'd say that this is a developer's mistake =/

PS: To drain huge amount of ETH (> 100 maybe) from that contract the Attack would need to drain more that 1 ETH per "inner" call, otherwise it will revert due to huge gas being spent on a single transaction.
So even in that case you could deploy another Attack contract that allow to drain 10, 100, 1000 ETH per call.

[maksym-nyrka]

@arturgontijo just wondering. From which kind of re-entrancy this forbid-evm-reentrancy feature will prevent?

[arturgontijo]

Hey, I'm not that familiar with its logic but checking the PR+code I'd say that it's a flag that blocks a call to Runner::execute() from being called again by its own execute_inner() stack.rs#L104.