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 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!