Root + Impact
Description
Vulnerability: BTT shares are ERC20 tokens and fully transferable via the inherited transfer() function. When a user transfers their shares to another address, the userToCountry mapping is NOT updated, creating a mismatch between who the protocol thinks won and who actually holds the shares. This results in permanent fund lockup.
function withdraw() external winnerSet {
if (
keccak256(abi.encodePacked(userToCountry[msg.sender])) !=
keccak256(abi.encodePacked(winner))
) {
revert didNotWin();
}
uint256 shares = balanceOf(msg.sender);
uint256 vaultAsset = finalizedVaultAsset;
uint256 assetToWithdraw = Math.mulDiv(shares, vaultAsset, totalWinnerShares);
_burn(msg.sender, shares);
IERC20(asset()).safeTransfer(msg.sender, assetToWithdraw);
}
Risk:
Likelihood: HIGH
Impact: HIGH
Original winner cannot withdraw despite selecting the correct team (has 0 shares after transfer)
Share recipient cannot withdraw despite holding shares (never joined event, userToCountry is empty)
Funds permanently locked in vault with no recovery mechanism
Complete loss of deposited assets for affected users
Proof of Concept
You may copy and paste the below POC to the existing test suite.
function test_ShareTransferBreaksWithdrawal() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
address bob = makeAddr("bob");
address alice = makeAddr("alice");
mockToken.mint(bob, 10 ether);
vm.startPrank(bob);
mockToken.approve(address(briVault), type(uint256).max);
uint256 bobShares = briVault.deposit(10 ether, bob);
uint256 vaultBalance = mockToken.balanceOf(address(briVault));
console.log("The Vault now holds this much balance:", vaultBalance);
console.log();
console.log("bob's shares after the deposit: ", bobShares);
briVault.joinEvent(10);
string memory bobsCountry = briVault.userToCountry(bob);
assertEq(bobsCountry, briVault.teams(10));
uint256 aliceSharesBefore = briVault.balanceOf(alice);
console.log("Alice shares: ", aliceSharesBefore);
console.log("Bob joined the event with index 10: ", bobsCountry);
console.log();
briVault.transfer(alice, bobShares);
uint256 bobsSharesAfter = briVault.balanceOf(bob);
uint256 aliceSharesAfter = briVault.balanceOf(alice);
console.log("bob's shares after the transfer: ", bobsSharesAfter);
console.log("Alice shares after the transfer: ", aliceSharesAfter);
assertEq(bobsSharesAfter, 0);
vm.stopPrank();
vm.warp(eventEndDate + 1);
vm.startPrank(owner);
briVault.setWinner(10);
vm.stopPrank();
vm.startPrank(bob);
briVault.withdraw();
uint256 bobBalanceAfterWithdraw = mockToken.balanceOf(bob);
console.log();
console.log("Bob's balance after withdrawal: ", bobBalanceAfterWithdraw);
console.log();
assertEq(bobBalanceAfterWithdraw, 0, "bob should recieve 0 tokens");
vm.stopPrank();
vm.startPrank(alice);
vm.expectRevert(abi.encodeWithSignature("didNotWin()"));
briVault.withdraw();
vm.stopPrank();
uint256 vaultBalanceAfter = mockToken.balanceOf(address(briVault));
console.log("The vault now still holds: ", vaultBalanceAfter, "but alice can't withdraw.");
}
Output Confirmation:
[PASS] test_ShareTransferBreaksWithdrawal() (gas: 1839597)
Logs:
The Vault now holds this much balance: 9850000000000000000
bob's shares after the deposit: 9850000000000000000
Alice shares: 0
Bob joined the event with index 10: Japan
bob's shares after the transfer: 0
Alice shares after the transfer: 9850000000000000000
Bob's balance after withdrawal: 0
The vault now still holds: 9850000000000000000 but alice can't withdraw.
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.64ms (888.59µs CPU time)
Recommended Mitigation
Use userSharesToCountry for withdrawal calculations instead of balanceOf():
function withdraw() external winnerSet {
// ...
- uint256 shares = balanceOf(msg.sender);
+ uint256 shares = userSharesToCountry[msg.sender][winnerCountryId];
uint256 vaultAsset = finalizedVaultAsset;
uint256 assetToWithdraw = Math.mulDiv(shares, vaultAsset, totalWinnerShares);
_burn(msg.sender, shares);
IERC20(asset()).safeTransfer(msg.sender, assetToWithdraw);
}
Option 2: Disable share transfers during active events:
+ function _beforeTokenTransfer(address from, address to, uint256 amount) internal virtual override {
+ if (from != address(0) && to != address(0)) { // Not mint/burn
+ if (block.timestamp >= eventStartDate && !_setWinner) {
+ revert("Shares locked during event");
+ }
+ }
+ super._beforeTokenTransfer(from, to, amount);
+ }
