Root + Impact

Description

In the withdrawWinnings() function, a user is able to wihdraw their winnings stored in pendingWinnings. In a normal working process, the winnings are securely transferred and the internal state is updated.

In this case however, the contract updates the user’s pendingWinnings after performing the external call to msg.sender. This creates a reentrancy vulnerability, where a malicious contract can reenter the same function. As a result, the attacker could repeatedly withdraw more than their actual winnings. Although the withdrawWinnings() function is protected by the nonReentrant modifier, this only prevents reentering the same function, it does not protect against cross-function reentrancy.

If another function accesses or modifies the same shared state (pendingWinnings) and is not protected by nonReentrant modifier, an attacker can exploit this gap by reentering through that unprotected function during the external call.

Risk

Likelihood:

• A user can deploy a contract with a fallback or receive function that reenters the withdrawWinnings() logic.

• Another function can also accesses and modify pendingWinnings, thus increasing the risk for reentrancy.

Impact:

• An attacker can repeatedly withdraw winnings before their balance is zeroed, draining the contract’s ETH.

Proof of Concept

Below is a simplified malicious contract that exploits the reentrancy vulnerability:

}

Recommended Mitigation

Use the checks-effects-interactions pattern. Update the internal state before making any external calls. This prevents reentrancy attacks even if the external call triggers a fallback function.