Proper check-effect-interactions pattern not followed all over the protocol

[codehawks-bot]

Proper check-effect-interactions pattern not followed all over the protocol

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L182C5-L192C6

Proper check-effect-interactions pattern not followed all over the protocol it make vulnerable to re-entrancy and no re-entrancy guard also used.

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol It is all over this file

Example:
Transferring the loanToken to lender before updating pools mapping

159:  IERC20(p.loanToken).transfer(
                p.lender,
                currentBalance - p.poolBalance
            );
        }

        emit PoolBalanceUpdated(poolId, p.poolBalance);

        if (pools[poolId].lender == address(0)) {
            // if the pool doesn't exist then create it
            emit PoolCreated(poolId, p);
        } else {
            // if the pool does exist then update it
            emit PoolUpdated(poolId, p);
        }

175:        pools[poolId] = p;
    }

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L159C13-L176C6
And
Updating the state before transferringFrom loanToken from lender to the protocol.

function addToPool(bytes32 poolId, uint256 amount) external {
        if (pools[poolId].lender != msg.sender) revert Unauthorized();
        if (amount == 0) revert PoolConfig();
        _updatePoolBalance(poolId, pools[poolId].poolBalance + amount);
        // transfer the loan tokens from the lender to the contract
        IERC20(pools[poolId].loanToken).transferFrom(
            msg.sender,
            address(this),
            amount
        );
    }

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L182C5-L192C6

Recommendation:
Use Proper check-effect-interactions pattern and use openzeppelin re-entrancy guard wherever transferring assets to outside the protocol and use safeTransfer and safeTransferFrom of openzeppelin instead of simple transfers functions of ERC20.

[PatrickAlphaC]

No proof here their is an actual bug, this is more of a pattern suggestion.

I agree they should follow CEI. Moving to informational finding.