Project
One World
NFTDeFi
15,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Unbounded for loop in `MembershipERC1155::burnBatchMultiple` function can cause DOS.

tdey

Summary

Unbounded for loop in MembershipERC1155::burnBatchMultiple function can cause DOS.

Vulnerability details

The MembershipERC1155::burnBatchMultiple function:

/// @notice Burn all tokens of multiple users
/// @param froms The addresses from which tokens will be burned
function burnBatchMultiple(address[] memory froms)
public
onlyRole(OWP_FACTORY_ROLE)
{
@> for(uint256 j = 0; j < froms.length; ++j){
//@audit unbounded loop. if batch size is too large DoS confirmed
for(uint256 i = 0; i < 7; ++i){
uint256 amount = balanceOf(froms[j], i);
if (amount > 0) {
burn_(froms[j], i, amount);
}
}
}
}

We can see from the function that, there is unbounded for loop

for(uint256 j = 0; j < froms.length; ++j)

Inside which there is another for loop hence the loop runs for j * 7 times for every value of j. As froms.length increases total number of runs would drastically increase.

POC

I have used foundry for writing the test, it is being installed using forge init --force we use the --force flag because it is a non-empty directory. The following test file MembershipERC1155Test includes the setup and the test function.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
​
import {Test} from "forge-std/Test.sol";
import {console} from "forge-std/console.sol";
import {console2} from "forge-std/console2.sol";
import {MembershipERC1155} from "../contracts/dao/tokens/MembershipERC1155.sol";
import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
import "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol";
​
contract MembershipERC1155Test is Test {
MembershipERC1155 public implementation;
MembershipERC1155 public token;
ProxyAdmin public proxyAdmin;
address public admin;
address public creator;
address public currency;
address[] public users;
​
uint256 public constant SAFE_BATCH_SIZE = 20;
uint256 public constant UNSAFE_BATCH_SIZE = 100; // Increased to make sure we hit the limit
uint256 public constant BLOCK_GAS_LIMIT = 30_000_000; // Polygon block gas limit
​
event GasUsed(string operation, uint256 gasUsed);
​
function setUp() public {
// Setup addresses
admin = address(this);
creator = makeAddr("creator");
currency = makeAddr("currency");
​
// Deploy implementation
implementation = new MembershipERC1155();
​
// Deploy ProxyAdmin
proxyAdmin = new ProxyAdmin(admin);
​
// Prepare initialization data
bytes memory initData = abi.encodeWithSelector(
MembershipERC1155.initialize.selector, "TestToken", "TEST", "ipfs://test-uri/", creator, currency
);
​
// Deploy proxy
TransparentUpgradeableProxy proxy =
new TransparentUpgradeableProxy(address(implementation), address(proxyAdmin), initData);
​
// Get token instance
token = MembershipERC1155(address(proxy));
​
// Create test users and mint tokens
for (uint256 i = 0; i < UNSAFE_BATCH_SIZE; i++) {
address user = makeAddr(string(abi.encodePacked("user", vm.toString(i))));
users.push(user);
​
// Mint tokens 0-6 to each user with large amounts
for (uint256 tokenId = 0; tokenId < 7; tokenId++) {
token.mint(user, tokenId, 1000); // Increased amount to consume more gas
}
}
}
​
function testBurnBatchSafe() public {
// Create batch of SAFE_BATCH_SIZE users
address[] memory safeBatch = new address[]();
for (uint256 i = 0; i < SAFE_BATCH_SIZE; i++) {
safeBatch[i] = users[i];
}
​
// Measure gas usage
uint256 gasBefore = gasleft();
token.burnBatchMultiple(safeBatch);
uint256 gasUsed = gasBefore - gasleft();
​
emit GasUsed("Safe batch burn", gasUsed);
console.log("Gas used for safe batch:", gasUsed);
​
// Verify burns
for (uint256 i = 0; i < safeBatch.length; i++) {
for (uint256 tokenId = 0; tokenId < 7; tokenId++) {
assertEq(token.balanceOf(safeBatch[i], tokenId), 0);
}
}
}
​
function testFailBurnBatchUnsafe() public {
// Create batch of UNSAFE_BATCH_SIZE users
address[] memory unsafeBatch = new address[]();
for (uint256 i = 0; i < UNSAFE_BATCH_SIZE; i++) {
unsafeBatch[i] = users[i];
}
​
token.burnBatchMultiple(unsafeBatch);
​
uint256 gasBefore = gasleft();
console2.log("Gas used", gasBefore);
​
uint256 exceedingGas = gasBefore - BLOCK_GAS_LIMIT;
console2.log("Gas used extra", exceedingGas);
​
if (gasBefore > BLOCK_GAS_LIMIT) revert();
}
​
}
​

When both the tests are ran in the test file we get the following outputs

  1. testFailBurnBatchUnsafe using forge test --mt testFailBurnBatchUnsafe -vvv

    Ran 1 test for test/MembershipERC1155Test.t.sol:MembershipERC1155Test
    [PASS] testFailBurnBatchUnsafe() (gas: 10998733)
    Logs:
    Gas used 1062726098
    Gas used extra 1032726098

  2. testBurnBatchSafe using forge test --mt testBurnBatchSafe -vvv

    Ran 1 test for test/MembershipERC1155Test.t.sol:MembershipERC1155Test
    [PASS] testBurnBatchSafe() (gas: 2019451)
    Logs:
    Gas used for safe batch: 2161044

In the test file there is two test to show what can be a approximate safe batch size and unsafe batch size. The test result clearly show that the unsafe batch size i.e. 100 here exceeds the 30mil block gas limit by a big margin.

But when we use a smaller batch size then the gas usage stays well within the block gas limit.

Impact

If batch size is large enough it will lead to DoS.
Impact : Medium , Likelihood : High (as it can occur with a batch size of just 100), but keeping severity as Medium as this is a admin controlled function.

Tools used

Manual review

Recommended mitigation

Have batch limits for example 20 as shown in the test. If large batches of tokens need to be burned, break large batches in smaller batches which would remain in the safe block gas limit.

Updates

Lead Judging Commences

0xbrivan2 Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Appeal created

tdey Submitter
7 months ago
0xbrivan2 Lead Judge
7 months ago
0xbrivan2 Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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