# Checks-effects-interations pattern is not always followed, which can be used to drain all tokens Crit/High · Three Sigma · Private DeFi access · 26th February, 2024 Finding 3S-SG-C03 of the Singularity security review. - Protocol: https://www.thesingularity.network/ - Report: /reports/singularity - Source: https://cdn.sanity.io/files/qoqld077/production/45b8aac56a2b8e3b557df4329ac9bf8220b64012.pdf --- ### Description [Here](https://fravoll.github.io/solidity-patterns/checks_effects_interactions.html) is a writeup about the pattern. Essentially state changes should be handled before interacting with external contracts (for example, when sending ETH via .call(""), to avoid reentrancy. [Here](https://github.com/portalgateme/darkpool-v1-zk-contracts-fork/blob/master/contracts/core/DarkpoolAssetManager.sol#L253-L254) is an example of the code not following it. _postWithdraw() should happen before sending ETH. So a user can call withdrawETH(), get execution when the ETH is sent, use the same note commitment (still not marked used) to swap on uniswap and double spend. This can be done in a loop to drain all tokens. ### Recommendation Ensure that the codebase follows this pattern. ### Status Addressed in [#8b300f0](https://github.com/portalgateme/darkpool-v1-zk-contracts-fork/commit/8b300f07a28f2c073154c8de3e9e90ba64ef1934). --- Related findings: - [Checks effects interactions pattern is now always followed](https://0xsimao.com/findings/orange-effects-interactions-now-followed): Orange Bridge - [Checks effects interactions pattern should always be used](https://0xsimao.com/findings/glacier-checks-effects-interactions-pattern): Glacier - [completeUnstake() not following checks-effects-interactions pattern](https://0xsimao.com/findings/metazero-ii-complete-unstake-following-interactions): MetaZero Staking - [CEI pattern is not followed in `GovernorTimelock::executeProposals()`](https://0xsimao.com/findings/maple-finance-ii-cei-followed-timelock-proposals): Maple Governor Timelock