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

`joinEvent` function cannot prevent DOS caused by excessive participating users

minos

joinEvent function cannot prevent DOS caused by excessive participating users

Description

  • When users call the joinEvent function to participate in an event, the protocol uses the usersAddress array to record participating users.

  • As the number of participants increases, the usersAddress array will grow larger and larger. When traversing this array (when calling setWinner), it will consume a huge amount of Gas.

function joinEvent(uint256 countryId) public {
// ...original code
@> usersAddress.push(msg.sender);
// ...original code
}
function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
// ...original code
@> _getWinnerShares();
// ...original code
}
function _getWinnerShares () internal returns (uint256) {
@> for (uint256 i = 0; i < usersAddress.length; ++i){
address user = usersAddress[i];
totalWinnerShares += userSharesToCountry[user][winnerCountryId]; //
}
return totalWinnerShares;
}

Risk

Likelihood:

  • It will occur once the administrator calls the setWinner function when there are enough participating users.

Impact:

  • The administrator needs to pay a relatively high Gas fee when calling the setWinner function.

  • If the number of participants is large enough, the Gas amount may even reach "the Gas limit of a single block on Ethereum", which will render the protocol ineffective.

Proof of Concept

  • Add the following function to test/BriVaultTest.t.sol and run forge test --mt test__joinEvent_withHugeUsers -vv

function test__joinEvent_withHugeUsers() public {
//uint256 gasCalc01 = gasleft();
// A large number of users deposit funds
uint256 userMaxCount = 6100;
address[] memory usersTmps = new address[](userMaxCount);
for(uint i=0; i<userMaxCount; i++) {
address userTmp = vm.addr(555 + i);
mockToken.mint(userTmp, 1 ether);
vm.startPrank(userTmp);
mockToken.approve(address(briVault), 1 ether);
briVault.deposit(1 ether, userTmp);
vm.stopPrank();
usersTmps[i] = userTmp;
}
//uint256 gasCalc02 = gasleft();
//console.log("gasCalc01 - gasCalc02", gasCalc01 - gasCalc02);
// Move timestamp to the block when the event just starts
vm.warp(eventStartDate + 0);
// A large number of users join the event
for(uint i=0; i<userMaxCount; i++) {
vm.startPrank(usersTmps[i]);
briVault.joinEvent(10);
vm.stopPrank();
}
uint256 gasCalc03 = gasleft();
//console.log("gasCalc02 - gasCalc03", gasCalc02 - gasCalc03);
// Move timestamp to after the event ends
vm.warp(eventEndDate + 1);
// The administrator sets the winner.
vm.prank(owner);
briVault.setWinner(9);
uint256 gasCalc04 = gasleft();
console.log("gasCalc03 - gasCalc04", gasCalc03 - gasCalc04);
}

  • Console output:

Ran 1 test for test/briVault.t.sol:BriVaultTest
[PASS] test__joinEvent_withHugeUsers() (gas: 858629828)
Logs:
gasCalc03 - gasCalc04 19970289
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.01s (1.01s CPU time)

  • The above console output shows that the Gas amount is close to 20 million.

Recommended Mitigation

  • Method 1: Limit the number of participants to a safe amount.

  • Method 2: Refactor the contract, replace address[] public usersAddress; with mapping (uint256 => uint256) public stakedAsset_KeyIsCountryIdx;, and adjust the contract according to the original logic.

Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Validated
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.

Support

FAQs

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

Give us feedback!