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

Permanent Fund Lock if Owner Never Calls setWinner

osamaebaid

Root + Impact

Description

  • Users deposit assets and join the event, staking funds in the vault until the tournament ends, after which winners can withdraw based on shares.

  • The owner must call setWinner after the event to set the winner and enable withdrawals via the winnerSet modifier.

  • The issue is that there is no timeout, fallback, or decentralized mechanism for finalization, so if the owner never calls setWinner (due to inactivity, loss of keys, or malice), the _setWinner flag remains false.

  • This permanently locks all deposited assets in the vault, as withdrawals require the winner to be set, with no way for users or others to recover funds

// In setWinner - owner-only, no fallback
function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
if (block.timestamp <= eventEndDate) {
revert eventNotEnded();
}
​
require(countryIndex < teams.length, "Invalid country index");
​
if (_setWinner) {
revert WinnerAlreadySet();
}
​
winnerCountryId = countryIndex;
winner = teams[countryIndex];
​
@> _setWinner = true; // @> Only set here; no alternative path or timeout
​
_getWinnerShares();
​
_setFinallizedVaultBalance();
​
emit WinnerSet (winner);
return winner;
}
​
// In withdraw - requires winnerSet modifier
function withdraw() external winnerSet {
// ... withdrawal logic ...
}
​
// Modifier requiring _setWinner
modifier winnerSet () {
@> if (_setWinner != true) { // @> Blocks forever if never set
revert winnerNotSet();
}
_;
}

Risk

Likelihood:Low

  • Owner becomes inactive or maliciously refuses to call setWinner after the event ends.

Impact:High

  • All user deposits remain locked indefinitely, leading to total funds loss.

  • No withdrawals or refunds possible, eroding trust in the contract and causing permanent asset inaccessibility.

Proof of Concept

  • Add function testPermanentFundLock to briVault.t.sol

  • run forge test --mt testPermanentFundLock

function testPermanentFundLock() public {
// User deposits and joins
vm.prank(user1);
mockToken.approve(address(briVault) , 1 ether);
vm.prank(user1);
briVault.deposit(1 ether, user1);
vm.prank(user1);
briVault.joinEvent(0);
​
// Advance time past event end (no setWinner called)
vm.warp(eventEndDate + 1 days);
​
// Attempt withdraw - fails because winner not set
vm.expectRevert("winnerNotSet()");
vm.prank(user1);
briVault.withdraw();
​
// Funds are locked: Vault has assets, but user can't access
assertGt(IERC20(address(mockToken)).balanceOf(address(briVault)), 0, "Assets locked in vault");
}

Recommended Mitigation

  • Add a timeout mechanism for refunds if winner not set

  • New function for refunds after timeout

​
+ uint256 public constant REFUND_TIMEOUT = 30 days; // e.g., 30 days after end
​
​
+ function emergencyRefund() external {
+ if (block.timestamp <= eventEndDate + REFUND_TIMEOUT) {
+ revert("Timeout not reached");
+ }
+ if (_setWinner) {
+ revert("Winner already set");
+ }
+ uint256 amount = stakedAsset[msg.sender];
+ require(amount > 0, "No stake");
+ stakedAsset[msg.sender] = 0;
+ uint256 shares = balanceOf(msg.sender);
+ _burn(msg.sender, shares);
+ IERC20(asset()).safeTransfer(msg.sender, amount);
+ }
Updates

Appeal created

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