Beatland Festival
First Flight #44
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: medium
Invalid

`transfer()` without Reentrancy Guard in `withdraw()` results in Potential Reentrancy

0xmaxi

Root + Impact

Description

  • The withdraw() function sends the entire contract balance to any address via transfer().

  • While transfer() is currently safe, switching to call() or future changes could introduce a reentrancy vector.

function withdraw(address target) external onlyOwner {
@>payable(target).transfer(address(this).balance);@>
}

Risk

Likelihood:

  • Future refactoring may change to .call{value:…} to avoid gas stipend issues.

  • Attackers could then reenter via fallback

Impact:

  • Draining of contract funds.


  • Corrupted contract state through reentrant calls.

Proof of Concept

contract Attacker {
FestivalPass public fp;
constructor(address _fp) { fp = FestivalPass(_fp); }
fallback() external payable {
fp.withdraw(address(this)); // reentrancy
}
function exploit() external {
fp.withdraw(address(this));
}
}
​

The attacker could reenter the FestivalPass contract by calling it's withdraw function using a fallback mechanism on a malicious contract.

Recommended Mitigation

- function withdraw(address target) external onlyOwner {
- payable(target).transfer(address(this).balance);
+ function withdraw(address target) external onlyOwner nonReentrant {
+ uint256 bal = address(this).balance;
+ (bool ok, ) = target.call{value: bal}("");
+ require(ok, "Transfer failed");
+ }

The nonReentrant modifier should be used for the withdraw function to prevent this kind of attack.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.