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

Vault direct donation causes unfair distribution

xrave110

Root + Impact

Description

  • Each depositor in case of win should get his deposits plus valid proportion of remaining tokens in the vault (besides other winners).

  • When someone donates directly to the vault without minting the share, increasing the total assets but not total supply of shares. This inflates the value of each share (exchange rate). When it happens between users the deposits, it leads to the unfair reward distribution for users who deposited after the donation.

function deposit(
uint256 assets,
address receiver
) public override returns (uint256) {
require(receiver != address(0));
​
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
​
uint256 fee = _getParticipationFee(assets);
// charge on a percentage basis points
if (minimumAmount + fee > assets) {
revert lowFeeAndAmount();
}
​
uint256 stakeAsset = assets - fee;
​
stakedAsset[receiver] = stakeAsset;
​
@> uint256 participantShares = _convertToShares(stakeAsset);
​
// if asset() will be ERC777 compatible and have callback it can call deposit again multiple times and cause that all funds go to participationFeeAddress
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:

  • When some direct donation happens and users deposit funds before and after donations

Impact:

  • Users deposited funds before direct donation will benefit more from potential win than users deposited after donation


Proof of Concept


Considering following scenario:

  1. User1 deposits small amount of assets

  2. Someone directly donated the vault without minting

  3. User2 deposits 1 ether

  4. User 3 deposits 1 ether

When the shares amount is logged, it occurs that user1 has unproportionally bigger amount of shares than the users deposited after donation (even if they deposited more).

// User1 deposits small amount
vm.startPrank(user1);
uint256 user1Amount = 0.001 ether;
mockToken.approve(address(briVault), user1Amount);
uint256 user1Shares = briVault.deposit(user1Amount, user1);
briVault.joinEvent(10);
uint256 balanceBeforuser1 = mockToken.balanceOf(user1);
vm.stopPrank();
​
// DONATION
vm.prank(user5);
mockToken.transfer(address(briVault), 5 ether);
​
// User2 deposits 1 ether
vm.startPrank(user2);
mockToken.approve(address(briVault), 1 ether);
uint256 user2Shares = briVault.deposit( 1 ether, user2);
briVault.joinEvent(10);
uint256 balanceBeforuser2 = mockToken.balanceOf(user2);
vm.stopPrank();
// User3 deposits 1 ether
vm.startPrank(user3);
mockToken.approve(address(briVault), 1 ether);
uint256 user3Shares = briVault.deposit( 1 ether, user3);
uint256 balanceBeforuser3 = mockToken.balanceOf(user3);
briVault.joinEvent(30);
vm.stopPrank();
​
console.log("Participant3 shares:", user3Shares);
console.log("Participant2 shares:", user2Shares);
console.log("Participant1 shares:", user1Shares); // << users shares are bigger than others even if his deposited amount is the smallest

Recommended Mitigation

Implement internal balance tracking separate from total assets.

​
Updates

Appeal created

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

Inflation attack

Support

FAQs

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

Give us feedback!