keep-network / keep-core

The smart contracts and reference client behind the Keep network
https://keep.network
MIT License
118 stars 73 forks source link

Inspect try-catch clauses in `solidity/random-beacon` contracts #3758

Open lukasz-zimnoch opened 8 months ago

lukasz-zimnoch commented 8 months ago

Several solidity/random-beacon contracts use try-catch blocks as part of their business logic. However, the EVM has a call stack depth limit equal to 1024. A third-party contract can leverage this limitation and force the try-catch-ed calls to revert unconditionally, by using recursion and letting those calls be executed at depth 1025. In such a case, the control flow is passed to the catch clauses which may lead to undesired side effects. Possibly affected contracts/libraries are:

This issue is about inspecting the existing try-catch blocks and fixing them in case they are prone to the aforementioned problem. A possible fix is requiring that the caller is an EOA using the following check:

require(msg.sender == tx.origin, "Not EOA");

Existing deployments of the solidity/random-beacon contracts have to be upgraded once the random beacon becomes operable and starts creating groups.

Note: A similar problem was already fixed in the WalletRegistry contract living in the solidity/ecdsa package: https://github.com/keep-network/keep-core/pull/3756