First Flight #2: Puppy Raffle

Summary

Not following Check-Effect-Interaction lead to reentrancy attack and result in lost of all the assets within the refund function.

Vulnerability Details

Inside the refund function, the sendValue function is placed before the update of the state variable - players . However, the function does not follow the check-effect-interaction pattern, the state variable update (effect) is placed after the interaction of external function (interaction). This leads to the reentrancy issue and can cause loss of all ethers locked in the contract.

The attacker can write a contract whose receive function invokes the refund function of the protocol. The attacker will reenter the refund function and withdraw the token until the balance become zero.

Impact

By the foundry testing framework, originally there are 100 ethers balance in the protocol and the hacker address. The hacker first use enterRaffle to register the address of attacker contract as player. After that, the attacker contract triggers the attack function and the exploit process starts, and the result shows the balance of the protocol becomes zero.

attacker contract：https://gist.github.com/e3eaff08f00200770df7d4ed3a8b951f.git

foundry contract： https://gist.github.com/535919db0f9fcb9f953bbdb17cecb1fb.git

Tools Used

manual review and foundry test

Recommendations

There are a few countermeasure to prevent reentrancy issues.

1. follow the Check-Effect-Interaction, moving players[playerIndex] = address(0) before payable(msg.sender).sendValue(entranceFee).

2. use nonReentrant modifier developed by oppenzeppelin within this function.

3. Adopt pull payment architecture designed by openzeppelin