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

Users can withdraw before the winner is finalized

eze001

Root + Impact

Description

  • Normally, users should only be able to withdraw their winnings after the tournament concludes and the winning team has been set by the contract owner.

  • The contract currently allows users to call withdrawWinnings() before the winner is finalized, which can lead to incorrect payouts or manipulation of funds.

// Root cause in the codebase with @> marks to highlight the relevant section
pragma solidity ^0.8.0;
​
contract PrematureWithdrawal {
mapping(address => uint256) public userShares;
mapping(address => uint256) public userTeam;
uint256 public winnerTeam;
bool public winnerSet;
​
function withdrawWinnings() external {
@> // no check if winner is set
@> uint256 payout = userShares[msg.sender];
@> userShares[msg.sender] = 0;
@> payable(msg.sender).transfer(payout);
}
}
​

Risk

Likelihood:

  • Occurs whenever a user calls withdrawWinnings() before the owner finalizes the winner.

  • Occurs whenever winnerSet or similar flags are not enforced before allowing withdrawals.

Impact:

  • Impact 1: Users may receive payouts prematurely, before a correct winner is determined, leading to misallocation of funds.

  • Impact 2: Attackers could exploit this to withdraw funds and cause shortages or griefing, compromising fairness.

Proof of Concept

Explanation:The PoC shows that a user can withdraw winnings before the winner is finalized. This allows premature payouts, misallocates funds, and could be exploited to disrupt fair distribution.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
contract PrematureWithdrawalPoC {
PrematureWithdrawal public vault;
​
constructor(PrematureWithdrawal _vault) { vault = _vault; }
​
function withdrawEarly() external {
vault.withdrawWinnings(); // withdraw before winner is set
}
}
​

Recommended Mitigation

Add a check to ensure the winner has been finalized before allowing withdrawals. This prevents premature withdrawals and ensures payouts are correct and fair.

- function withdrawWinnings() external {
- uint256 payout = userShares[msg.sender];
- userShares[msg.sender] = 0;
- payable(msg.sender).transfer(payout);
- }
+ function withdrawWinnings() external {
+ require(winnerSet, "winner not finalized");
+ uint256 payout = userShares[msg.sender];
+ userShares[msg.sender] = 0;
+ payable(msg.sender).transfer(payout);
+ }
​
Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!