BriVault
First Flight #52
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Share transfer undermining the game's fairness

jericho

Root + Impact

Description

  • The contract inherits from ERC4626 which implements standard ERC20 transfer functionality, allowing shares to be freely transferred between addresses at any time. This completely breaks the game's fairness model because users can transfer their shares to winning country addresses after the winner is known, allowing them to claim a disproportionate share of the rewards.

contract BriVault is ERC4626, Ownable { // @> Inherits ERC20 transfer functionality
// ... no transfer restrictions implemented ...
function withdraw() external winnerSet {
if (keccak256(abi.encodePacked(userToCountry[msg.sender])) != keccak256(abi.encodePacked(winner))) {
revert didNotWin();
}
uint256 shares = balanceOf(msg.sender); // @> Checks current balance, not original participation
// ... withdrawal logic based on current share balance ...
}
}

Risk

Likelihood:

  • Shares can be transferred at any time without restrictions

  • After the winner is announced but before withdrawals, users from losing countries can transfer shares to winning country addresses

  • This can be done through direct transfers or approved transfers via transferFrom

  • The vulnerability is easily exploitable with minimal technical knowledge


Impact:

  • Complete breakdown of game fairness - rewards are distributed based on current share ownership, not original participation

  • Economic exploitation - losing participants can sell their shares to winning participants after results are known

  • Centralization risk - whales can accumulate winning country shares after the event ends

  • Loss of trust in the protocol - the core game mechanics become meaningless


Proof of Concept

User3 (loser) transfers all shares to User1 (winner),User1 withdraws - gets rewards for User3's shares too! User2 cannot withdraw because their shares weren't increased

function test_shareTransferBreaksFairness() public {
// Setup: Three users deposit and join different countries
vm.prank(owner);
briVault.setCountry(countries);
​
// User3 joins country 1 (will lose)
vm.startPrank(user3);
mockToken.approve(address(briVault), 1 ether);
briVault.deposit(1 ether, user3);
briVault.joinEvent(1);
vm.stopPrank();
​
// User2 and User1 join country 2 (will win)
vm.startPrank(user2);
mockToken.approve(address(briVault), 1 ether);
briVault.deposit(1 ether, user2);
briVault.joinEvent(2);
vm.stopPrank();
​
vm.startPrank(user1);
mockToken.approve(address(briVault), 1 ether);
briVault.deposit(1 ether, user1);
briVault.joinEvent(2);
vm.stopPrank();
​
// Event ends, winner is set to country 2
vm.warp(eventEndDate + 1);
vm.prank(owner);
briVault.setWinner(2);
​
// ATTACK: User3 (loser) transfers all shares to User1 (winner)
vm.startPrank(user3);
briVault.transfer(user1, briVault.balanceOf(user3));
vm.stopPrank();
​
// User1 withdraws - gets rewards for User3's shares too!
vm.prank(user1);
briVault.withdraw();
​
// User2 cannot withdraw because their shares weren't increased
vm.expectRevert();
vm.prank(user2);
briVault.withdraw();
// Result: User1 gets 2/3 of the pool instead of 1/2
// User3 recovers value by selling shares to User1
// User2 gets nothing despite being on winning team
}

Recommended Mitigation

Implement transfer restrictions during critical periods:

diff --git a/src/briVault.sol b/src/briVault.sol
index bf96d73..a06d8b8 100644
--- a/src/briVault.sol
+++ b/src/briVault.sol
@@ -52,7 +52,8 @@ contract BriVault is ERC4626, Ownable {
// Array of teams
string[48] public teams;
+ bool public transfersEnabled;
// Error Logs
error eventStarted();
@@ -66,6 +67,8 @@ contract BriVault is ERC4626, Ownable {
error eventNotStarted();
error WinnerAlreadySet();
error limiteExceede();
+ error TransfersDisabled();
event deposited (address indexed _depositor, uint256 _value);
event CountriesSet(string[48] country);
​
@@ -89,6 +92,7 @@ contract BriVault is ERC4626, Ownable {
participationFeeAddress = _participationFeeAddress;
minimumAmount = _minimumAmount;
_setWinner = false;
+ transfersEnabled = false;
}
modifier winnerSet () {
​
@@ -139,6 +143,21 @@ contract BriVault is ERC4626, Ownable {
}
+ // Override transfer functions with restrictions
+ function _update(address from, address to, uint256 value) internal override {
+ if (from != address(0) && to != address(0)) { // Skip for mint/burn
+ if (!transfersEnabled) {
+ revert TransfersDisabled();
+ }
+ }
+ super._update(from, to, value);
+ }
+
+ // Optional: Allow owner to enable transfers earlier if needed
+ function enableTransfers() external onlyOwner {
+ transfersEnabled = true;
+ }
+
/**
* @notice sets the finalized vault balance
*/
Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Unrestricted ERC4626 functions

Support

FAQs

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

Give us feedback!