Summary

The transferErcTokens function modifies internal balances before executing the external transferFrom call, violating the Checks-Effects-Interactions (CEI)

Vulnerability Details

https://github.com/Cyfrin/2025-01-pieces-protocol/blob/4ef5e96fced27334f2a62e388a8a377f97a7f8cb/src/TokenDivider.sol#L179-L207

Medium

Below is the function which is affected

First state change: Decreases sender's balance before verifying transfer success
If transfer fails, this reduction becomes permanent incorrectly

Second state change: Increases recipient's balance before verifying transfer

External call made after state changes. If this fails, previous state changes aren't reverted

In the contract, the sender’s token balance (balances[msg.sender][tokenInfo.erc20Address]) is reduced before ensuring that the actual token transfer succeeds.
If the transfer fails (e.g., due to insufficient allowance or network issues), the balance reduction remains permanent, even though the tokens weren’t sent. Similarly, the recipient’s token balance (balances[to][tokenInfo.erc20Address]) is increased before the external transfer is verified.
If the transfer fails, the recipient’s balance incorrectly shows tokens they never received. The actual token transfer (IERC20(tokenInfo.erc20Address).transferFrom) happens after the state changes. If this call fails, the balances are already modified incorrectly, and there’s no mechanism to automatically reverse these changes.

Impact

1. If the external transferFrom call fails, Internal balances become permanently out of sync with actual token balances if transfer fails

2. Users could lose access to their tokens due to incorrect balance records

3. Tokens are essentially lost in the system due to incorrect state updates.

Recommendation

The Contract should Perform the token transfer first using IERC20.transferFrom and then Only update the balances after verifying the transfer succeeded.