Description

Functions KittyPool::meowintKittyCoin and KittyPool::whiskdrawMeowllateral do not follow the Checks-Effects-Interactions pattern, as require statements are placed after some state changes have occurred. The require statements that checks for sufficient collateral are placed at the end of the functions. This means that the state changes, e.g. updating kittyCoinMeownted mapping and minting KittyCoins in KittyPool::meowintKittyCoin happen before the validation of the user’s collateral. The Checks-Effects-Interactions pattern dictates that checks (require statements) should occur before any state modifications or external interactions to prevent unintended consequences or vulnerabilities.

Impact

Lets demonstrate the effect of a misfollowed Checks-Effects-Interactions pattern vulnerability on the KittyPool::meowintKittyCoin function.
If the _hasEnoughMeowllateral function returns false in the require statement, the transaction will revert, but the minting of coins might have already occurred if there are issues with state rollback, so the contract might enter an inconsistent state. This can lead to inflation of KittyCoins, potentially causing financial instability in the ecosystem. If a transaction reverts after performing unnecessary state changes, it can also waste gas, making the operation costlier for users and reducing the overall efficiency of the contract.

Tools Used

Manual review, vscode

Recommended Mitigation

Consider reordering the KittyPool::meowintKittyCoin and KittyPool::whiskdrawMeowllateral functions to ensure checks are performed first: