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

Critical: Multiple `deposit()` Calls Overwrite `stakedAsset`, Causing Under-Payment

mnusurov

Critical: Multiple deposit() Calls Overwrite stakedAsset, Causing Under-Payment

Description

  • deposit() overwrites stakedAsset[receiver] with stakeAsset = assets - fee.

  • User can call deposit() multiple times → last deposit recoded.

function deposit(uint256 assets, address receiver) public override returns (uint256) {
...
uint256 stakeAsset = assets - fee;
stakedAsset[receiver] = stakeAsset; // @> Overwrites previous staked amount!
...
}

Risk

Likelihood:

  • After first deposit() — call deposit() again before eventStartDate.

Impact:

  • User loses all but last deposit on cancelParticipation().


Funds locked — no recovery.

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/PocC09.t.sol

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
import {BriVaultExploitTest, Math} from "./BriVaultExploitTest.t.sol";
​
contract PocC09 is BriVaultExploitTest {
using Math for uint256;
​
function test_criticalDepositOverwritesStakedAsset() external {
// Step 1: Second deposit — (overwrites stakedAsset)
_deposit(user1, user1, USER1_AMOUNT);
​
// Step 2: User cancels — expects ~USER1_AMOUNT * 2 ETH back, gets ~USER1_AMOUNT wei
uint256 fairBalance = USER1_AMOUNT.mulDiv(AFTER_FEE, FEE_BASE) * 2;
vm.prank(user1);
vault.cancelParticipation();
uint256 user1Balance = token.balanceOf(user1);
emit log_named_uint("User1 expected fair", fairBalance);
emit log_named_uint("User1 actually got", user1Balance);
assertLt(user1Balance, fairBalance, "User1 got less than fair balance");
}
}

Recommended Mitigation

function deposit(uint256 assets, address receiver) public override returns (uint256) {
...
uint256 stakeAsset = assets - fee;
​
- stakedAsset[receiver] = stakeAsset;
+ stakedAsset[receiver] += stakeAsset;
​
uint256 participantShares = _convertToShares(stakedAsset[receiver]);
...
}
Updates

Appeal created

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

`stakedAsset` Overwritten on Multiple Deposits

Vault tracks only a single deposit slot per user and overwrites it on every call instead of accumulating the total.

Support

FAQs

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

Give us feedback!