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

Users Unable to Update Their Tier Due to Insufficient Tokens, Resulting in a DoS Vulnerability from Under/Overflow

theirrationalone

Summary

In MembershipFactory.sol::MembershipFactory, the function MembershipFactory::upgradeTier is intended to allow users to upgrade their tier within a sponsored DAO. However, this function currently burns 2 DAOMembership tokens rather than 1. Was this intentional? To properly facilitate the upgrade, the protocol should only burn one token and perhaps charge a fee for the tier upgrade.

/// @notice Allows users to upgrade their tier within a sponsored DAO
/// @param daoMembershipAddress The address of the DAO membership NFT
/// @param fromTierIndex The current tier index of the user
function upgradeTier(address daoMembershipAddress, uint256 fromTierIndex) external {
require(daos[daoMembershipAddress].daoType == DAOType.SPONSORED, "Upgrade not allowed.");
require(daos[daoMembershipAddress].noOfTiers >= fromTierIndex + 1, "No higher tier available.");
// @info: missing storage update, also mentioned in cyrfin's private audit
// @info: Burning may fail here
@> IMembershipERC1155(daoMembershipAddress).burn(_msgSender(), fromTierIndex, 2);
-----------------------------------------------------------------------------------^
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), fromTierIndex - 1, 1);
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, fromTierIndex - 1);
}

Vulnerability Details

  • A user in a sponsored DAO attempts to upgrade their tier.

  • The user calls the upgradeTier function.

  • The user has minted only 1 token in the current tier (fromTierIndex).

  • The upgradeTier call fails due to insufficient token balance, causing an under/overflow error.

Consider the following test case that serves as a proof of concept:

  1. Convert the project to a Foundry-based structure.

  2. Install all necessary dependencies.

  3. Configure remappings as needed.

  4. Create a file named MembershipERC1155Test.sol in the test folder.

  5. Add the following code to MembershipERC1155Test.sol:

// SPDX-License-Identifier: MIT
​
pragma solidity 0.8.22;
​
import {Test, console} from "forge-std/Test.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
​
import {MembershipERC1155} from "../src/dao/tokens/MembershipERC1155.sol";
import {MembershipFactory} from "../src/dao/MembershipFactory.sol";
import {CurrencyManager} from "../src/dao/CurrencyManager.sol";
import {TierConfig, DAOConfig, DAOInputConfig, DAOType} from "../src/dao/libraries/MembershipDAOStructs.sol";
​
import {CurrencyMock} from "./mock/CurrencyMock.sol";
import {OWPWalletMock} from "./mock/OWPWalletMock.sol";
​
contract MembershipERC1155Test is Test {
CurrencyMock currency;
address CREATOR = makeAddr("CREATOR");
address CURRENCY_MANAGER_ADMIN = makeAddr("CURRENCY_MANAGER_ADMIN");
address membershipDAOCreatorAndJoiner = makeAddr("membershipDAOCreatorAndJoiner");
address MEMBERSHIP_FACTORY_ADMIN_AND_EXTERNAL_CALLER = makeAddr("MEMBERSHIP_FACTORY_ADMIN_AND_EXTERNAL_CALLER");
address OWP_FACTORY_AND_DEFAULT_ADMIN = makeAddr("OWP_FACTORY_AND_DEFAULT_ADMIN");
string BASE_URI = "https://one-world.com/uri/";
​
CurrencyManager currencyManager;
OWPWalletMock owpWalletMock;
MembershipFactory membershipFactory;
MembershipERC1155 membershipTokenImpl;
​
function setUp() public {
currency = new CurrencyMock();
​
vm.startPrank(CURRENCY_MANAGER_ADMIN);
currencyManager = new CurrencyManager();
currencyManager.addCurrency(address(currency));
vm.stopPrank();
​
vm.startPrank(OWP_FACTORY_AND_DEFAULT_ADMIN);
membershipTokenImpl = new MembershipERC1155();
vm.stopPrank();
​
owpWalletMock = new OWPWalletMock();
​
vm.startPrank(MEMBERSHIP_FACTORY_ADMIN_AND_EXTERNAL_CALLER);
membershipFactory = new MembershipFactory(
address(currencyManager), address(owpWalletMock), BASE_URI, address(membershipTokenImpl)
);
vm.stopPrank();
}
​
function baseFunction() public returns (MembershipERC1155) {
TierConfig memory tierConf_one = TierConfig({amount: 10, price: 100, power: 2, minted: 0});
TierConfig memory tierConf_two = TierConfig({amount: 10, price: 200, power: 3, minted: 0});
TierConfig;
tiersArr[0] = tierConf_one;
tiersArr[1] = tierConf_two;
tiersArr[2] = tierConf_two;
tiersArr[3] = tierConf_two;
tiersArr[4] = tierConf_two;
tiersArr[5] = tierConf_two;
tiersArr[6] = tierConf_two;
​
DAOInputConfig memory daoInputConf = DAOInputConfig({
ensname: "anyENSNAME",
daoType: DAOType.SPONSORED,
currency: address(currency),
maxMembers: 70,
noOfTiers: tiersArr.length
});
​
vm.startPrank(membershipDAOCreatorAndJoiner);
address daoMembertokenProxyAddress = membershipFactory.createNewDAOMembership(daoInputConf, tiersArr);
vm.stopPrank();
​
MembershipERC1155 daoMemberToken = MembershipERC1155(daoMembertokenProxyAddress);
​
return daoMemberToken;
}
​
function testusersFailToUpdateTierDueToUnWantedTokenBurn() public {
// A user creates a DAO
MembershipERC1155 daoMemberToken = baseFunction();
​
vm.expectRevert();
membershipFactory.upgradeTier(address(daoMemberToken), 6);
​
vm.startPrank(membershipDAOCreatorAndJoiner);
currency.mint(membershipDAOCreatorAndJoiner, 500);
currency.approve(address(membershipFactory), 200);
membershipFactory.joinDAO(address(daoMemberToken), 6);
vm.expectRevert();
membershipFactory.upgradeTier(address(daoMemberToken), 6);
vm.stopPrank();
}
}

  1. Create a mock directory in the test folder.

  2. Inside mock, add the files CurrencyMock.sol and OWPWalletMock.sol.

  3. Implement the required mock contracts within these files.

  4. Open a terminal and run the following command to execute the test:

forge test --mt testusersFailToUpdateTierDueToUnWantedTokenBurn

  1. Review the test logs:

[⠊] Compiling...
[⠘] Compiling 1 files with Solc 0.8.22
[⠃] Solc 0.8.22 finished in 3.63s
Compiler run successful!
​
Ran 1 test for test/MembershipERC1155Test.t.sol:MembershipERC1155Test
[PASS] testusersFailToUpdateTierDueToUnWantedTokenBurn() (gas: 1736888)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.97ms (896.70µs CPU time)
​
Ran 1 test suite in 9.19ms (3.97ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

The test successfully passes, confirming the oversight.

Impact

  • Users attempting to upgrade their tier in the DAO would require a balance of at least 2 DAO tokens.

  • If a user only has 1 token in the current tier, they would encounter a DoS vulnerability due to an under/overflow when attempting to burn tokens they do not possess.

Tools Used

Manual Review, Foundry

Recommendations

To resolve this issue,

  • The protocol should burn only one token during a tier upgrade and consider implementing a fee mechanism for the tier upgrade process if needed.

  • If it's intentional then Please implement a check that should revert if users have less than 2 tokens or if they're not eligible to upgrade.

Updates

Lead Judging Commences

0xbrivan2 Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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