Submission Details
Severity:
There is a possibility that DDoS attacks in _getWinnerShares could result in a higher consumption of GUS.
Root + Impact
Description
-
Under normal circumstances, the _getWinnerShares() function is used to calculate the total share of the winning country/region. It iterates through all the participant addresses and accumulates the corresponding shares.
-
The problem is that this function uses an unrestricted for loop to traverse the entire usersAddress array. If the array length is very large (for example, with thousands or tens of thousands of addresses), this loop will consume a significant amount of gas, potentially causing the transaction to fail or exceeding the gas limit of the block.
function _getWinnerShares () internal returns (uint256) {
// Internal function: Calculate the total share of the winner
// Return: Total share of the winner
@> for (uint256 i = 0; i < usersAddress.length; ++i){
// Traverse all user addresses
address user = usersAddress[i];
totalWinnerShares += userSharesToCountry[user][winnerCountryId];
}
return totalWinnerShares;
}
Risk
Likelihood:
-
Reason 1 When the length of the usersAddress array becomes extremely large (for example, more than 1000 addresses), this loop operation will consume a significant amount of gas.
-
Reason 2 When setting the winner (by calling the setWinner function), this high-gas-consumption loop operation will definitely be triggered.
Impact:
-
Impact 1 The transaction may fail due to exceeding the gas limit of the block, resulting in the inability to determine the winner and complete the activity.
-
Impact 2 Even if the transaction is successful, it will result in extremely high gas fees, and a large amount of Ether may be required to execute this operation.
Proof of Concept
// SPDX-License-Identifier: MIT
import "forge-std/Test.sol";
import "../src/briVault.sol";
import "forge-std/console.sol";
contract BriVaultDOSGasTest is Test {
MockERC20 public token;
address public owner;
uint256 public constant INITIAL_SUPPLY = 10_000_000 ether;
uint256 public constant EVENT_START = 1 days;
uint256 public constant EVENT_END = 8 days;
uint256 public constant PARTICIPATION_FEE = 100; // 1%
uint256 public constant MIN_AMOUNT = 1 ether;
function setUp() public {
owner = address(this); // Use the test contract address as the owner
token = new MockERC20("Test Token", "TTK", INITIAL_SUPPLY);
// Prepare sufficient tokens for the test
token.transfer(address(this), INITIAL_SUPPLY / 2);
}
// Focus on presenting the gas consumption comparison test of the _getWinnerShares function
function testSetWinnerGasScaling() public {
// Test different user counts: 100, 200, and 500 users
uint256[] memory userCounts = new uint256[](3);
userCounts[0] = 100;
userCounts[1] = 200;
userCounts[2] = 500;
// Store gas consumption for each user count
uint256[] memory gasConsumptions = new uint256[](3);
for (uint i = 0; i < userCounts.length; i++) {
uint256 count = userCounts[i];
// Create new test instance
BriVault newVault = new BriVault(
IERC20(address(token)),
PARTICIPATION_FEE,
block.timestamp + EVENT_START,
owner,
MIN_AMOUNT,
block.timestamp + EVENT_END
);
// Set country list
string[48] memory countries;
for (uint j = 0; j < 48; j++) {
countries[j] = string(abi.encodePacked("Country", vm.toString(j)));
}
newVault.setCountry(countries);
// Advance time to just before event starts
vm.warp(block.timestamp + EVENT_START - 1 hours);
// Create users and have them join the event - all join the same country (0)
for (uint j = 0; j < count; j++) {
address user = address(uint160(3000 + j));
token.transfer(user, 10 ether);
vm.prank(user);
token.approve(address(newVault), 10 ether);
vm.prank(user);
newVault.deposit(2 ether, user);
vm.prank(user);
newVault.joinEvent(0); // All users join the same country
}
// Advance time to after event end
vm.warp(block.timestamp + EVENT_END + 1 hours);
// Measure gas consumption of setWinner - this triggers _getWinnerShares
uint256 gasBefore = gasleft();
newVault.setWinner(0);
uint256 gasUsed = gasBefore - gasleft();
gasConsumptions[i] = gasUsed;
}
// Only retain the key gas consumption comparison output
console.log("Gas consumption change when doubling users (100 to 200):", gasConsumptions[1] * 100 / gasConsumptions[0], "%");
console.log("Gas consumption change when 5x users (100 to 500):", gasConsumptions[2] * 100 / gasConsumptions[0], "%");
// Ensure that gas consumption increases as the number of users grows.
assertGt(gasConsumptions[1], gasConsumptions[0]);
assertGt(gasConsumptions[2], gasConsumptions[1]);
}
}
// Simplified ERC20 Simulation Contract
contract MockERC20 {
string public name;
string public symbol;
uint8 public decimals = 18;
uint256 public totalSupply;
mapping(address => uint256) public balanceOf;
mapping(address => mapping(address => uint256)) public allowance;
constructor(string memory _name, string memory _symbol, uint256 _initialSupply) {
name = _name;
symbol = _symbol;
totalSupply = _initialSupply;
balanceOf[msg.sender] = _initialSupply;
}
function transfer(address to, uint256 amount) public returns (bool) {
balanceOf[msg.sender] -= amount;
balanceOf[to] += amount;
return true;
}
function approve(address spender, uint256 amount) public returns (bool) {
allowance[msg.sender][spender] = amount;
return true;
}
function transferFrom(address from, address to, uint256 amount) public returns (bool) {
allowance[from][msg.sender] -= amount;
balanceOf[from] -= amount;
balanceOf[to] += amount;
return true;
}
}
Recommended Mitigation
// 1. Add the countryTotalShares mapping to the declaration section at the top of the contract.
+ mapping(uint256 => uint256) public countryTotalShares;
// 2. Modify the _getWinnerShares function to avoid traversing all users
- function _getWinnerShares () internal returns (uint256) {
- for (uint256 i = 0; i < usersAddress.length; ++i){
- address user = usersAddress[i];
- totalWinnerShares += userSharesToCountry[user][winnerCountryId];
- }
- return totalWinnerShares;
- }
+ function _getWinnerShares () internal returns (uint256) {
+ // Directly obtain the total share of the winning country from the pre-maintained mapping.
+ totalWinnerShares = countryTotalShares[winnerCountryId];
+ return totalWinnerShares;
+ }
// 3. Modify the `joinEvent` function to update the total share of the country when a user joins.
- function joinEvent(uint256 countryId) public {
- uint256 participantShares = balanceOf(msg.sender);
- userSharesToCountry[msg.sender][countryId] = participantShares;
+ function joinEvent(uint256 countryId) public {
+ uint256 participantShares = balanceOf(msg.sender);
+ userSharesToCountry[msg.sender][countryId] = participantShares;
+ countryTotalShares[countryId] += participantShares;
// 4. Modify the "cancelParticipation" function to update the total share of the country when the user cancels their participation.
- function cancelParticipation () public {
- uint256 shares = balanceOf(msg.sender);
+ function cancelParticipation () public {
+ uint256 shares = balanceOf(msg.sender);
+ // Search for the countries that the user has joined and update the total share for each country
+ for (uint256 i = 0; i < teams.length; ++i) {
+ if (userSharesToCountry[msg.sender][i] > 0) {
+ countryTotalShares[i] -= userSharesToCountry[msg.sender][i];
+ break;
+ }
+ }
+ _burn(msg.sender, shares);
Updates
Appeal created
bube 16 days ago
Submission Judgement Published Assigned finding tags:
Unbounded Loop in _getWinnerShares Causes Denial of Service
The _getWinnerShares() function is intended to iterate through all users and sum their shares for the winning country, returning the total.
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.