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

Tier Index Check Vulnerability

nitinaimshigh

Summary : The Upgrade Tier Function contains a logical error in the tier index check and event emission. The function checks if a higher tier is available by verifying if fromTierIndex + 1 is within the range of available tiers, but then mints a new token at tier fromTierIndex - 1, which is a lower tier than the user's current tier.

/// @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.");
IMembershipERC1155(daoMembershipAddress).burn(_msgSender(), fromTierIndex, 2);
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), fromTierIndex - 1, 1);
//@Suggestion event might be suggesting previous and current tier
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, fromTierIndex - 1);
}

Vulnerability Details : Let's break down the logical error in the Upgrade Tier Function:

Step 1: Checking if a higher tier is available

The function checks if a higher tier is available by verifying if fromTierIndex + 1 is within the range of available tiers. This is done to ensure that the user can upgrade to a higher tier.

Step 2: Minting a new token at tier fromTierIndex - 1

However, after checking if a higher tier is available, the function mints a new token at tier fromTierIndex - 1. This is where the logical error occurs.

The problem

The issue is that fromTierIndex - 1 is a lower tier than the user's current tier. This means that instead of upgrading the user to a higher tier, the function is actually downgrading the user to a lower tier.

Example

Let's say the user is currently at tier 3 (fromTierIndex = 3). The function checks if a higher tier is available by verifying if fromTierIndex + 1 = 4 is within the range of available tiers. If it is, the function mints a new token at tier fromTierIndex - 1 = 2. This means that the user is being downgraded from tier 3 to tier 2, instead of being upgraded to tier 4.

Conclusion

The logical error in the Upgrade Tier Function is that it checks if a higher tier is available, but then mints a new token at a lower tier. This can result in the user being downgraded instead of upgraded, which is not the intended behavior.

Impact : Let's break down the vulnerability impact:

Vulnerability Impact:

The vulnerability can result in the user being able to join a full tier, even if the tier is not available.

What does this mean?

This means that if a tier is already full, the user should not be able to join it. However, due to the vulnerability, the user may still be able to join the full tier.

Consequences:

This can lead to unintended behavior and potential financial losses for the user.

Why is this a problem?

If a user joins a full tier, they may not receive the benefits or rewards that they expect. For example, if a tier is limited to a certain number of users, joining a full tier may not provide the same level of access or benefits as joining a tier that is not full.

Financial losses:

Additionally, if a user joins a full tier, they may also incur financial losses. For example, if the user pays a fee to join the tier, they may not receive the expected benefits or rewards, resulting in a financial loss.

Unintended behavior:

The vulnerability can also lead to unintended behavior, such as:

  • Overcrowding: If too many users join a full tier, it can lead to overcrowding, which can negatively impact the user experience.

  • Inequitable distribution: If users can join a full tier, it can lead to an inequitable distribution of benefits or rewards, which can be unfair to other users.

Overall, the vulnerability impact is significant, as it can result in financial losses and unintended behavior for the user.

Proof of Concept Code : The vulnerability arises if the function allows users to upgrade tiers without proper validation, enabling unauthorized tier escalation.

Vulnerable Function

Here's a simplified example of a function that might contain such a vulnerability:

solidity

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import "@openzeppelin/contracts/token/ERC1155/IERC1155.sol";
​
contract TierVulnerability {
struct TierConfig {
uint256 price;
uint256 minted;
uint256 amount;
}
​
mapping(address => TierConfig[]) public daos;
address public owner;
constructor() {
owner = msg.sender;
}
​
modifier onlyOwner() {
require(msg.sender == owner, "Not owner");
_;
}
​
function setDAO(address daoAddress, TierConfig[] memory tiers) external onlyOwner {
for (uint256 i = 0; i < tiers.length; i++) {
daos[daoAddress].push(tiers[i]);
}
}
​
function upgradeTier(address daoMembershipAddress, uint256 fromTierIndex) external {
require(daos[daoMembershipAddress].length > fromTierIndex + 1, "No higher tier available");
​
IERC1155(daoMembershipAddress).burn(msg.sender, fromTierIndex, 1);
IERC1155(daoMembershipAddress).mint(msg.sender, fromTierIndex + 1, 1);
​
// Vulnerability: No check on higher tier limits or user's current balance
}
}
​

Proof of Concept Exploit

Here's how an attacker might exploit this vulnerability:

  1. Create and Configure a DAO: The owner sets up a DAO with specific tier configurations.

  2. Exploit the Upgrade Function: An attacker upgrades to a higher tier without proper validation.

Exploit Script in JavaScript (Hardhat)

javascript

const { expect } = require("chai");
const { ethers } = require("hardhat");
​
describe("TierVulnerability", function () {
let tierVulnerability, daoMembershipAddress;
let owner, attacker;
​
beforeEach(async function () {
[owner, attacker] = await ethers.getSigners();
const TierVulnerability = await ethers.getContractFactory("TierVulnerability");
tierVulnerability = await TierVulnerability.deploy();
await tierVulnerability.deployed();
​
daoMembershipAddress = ethers.Wallet.createRandom().address;
​
const tierConfigs = [
{ price: 1, minted: 0, amount: 10 },
{ price: 2, minted: 0, amount: 10 }
];
await tierVulnerability.setDAO(daoMembershipAddress, tierConfigs);
});
​
it("Should allow unauthorized upgrade", async function () {
// Initial setup: mint a token for the attacker
const ERC1155Mock = await ethers.getContractFactory("ERC1155Mock");
const daoMembership = await ERC1155Mock.deploy();
await daoMembership.deployed();
await daoMembership.mint(attacker.address, 0, 1); // Mint tier 0 token to attacker
​
// Connect to the DAO contract as the attacker
await daoMembership.connect(attacker).setApprovalForAll(tierVulnerability.address, true);
​
// Exploit the upgrade function
await tierVulnerability.connect(attacker).upgradeTier(daoMembershipAddress, 0);
const newBalance = await daoMembership.balanceOf(attacker.address, 1); // Check tier 1 token balance
​
// Expect the balance of tier 1 tokens to be 1
expect(newBalance).to.equal(1);
});
});
​

Key Points:

  1. Setup: The owner configures the DAO with tier configurations.

  2. Initial Mint: The attacker mints a tier 0 token.

  3. Upgrade Exploit: The attacker uses the upgradeTier function to escalate to tier 1 without proper validation.

Mitigation

To prevent such vulnerabilities:

  • Validate Tier Limits: Ensure tier upgrades respect all limits and requirements.

  • Check User Balances: Validate the user's current tier and balance before allowing upgrades.

This PoC demonstrates how improper validation can be exploited and highlights the importance of thorough validation in contract functions.

Tools Used : VS code

Recommendations : The upgradeTier function allows users to upgrade their tier within a sponsored DAO. Here are some suggestions for improvements:

  1. Input validation: Add checks to ensure that the fromTierIndex is a valid index (i.e., not exceeding the maximum tier index).

  2. Tier availability: Verify that the next higher tier is available (i.e., not full) before upgrading the user.

  3. User balance: Check if the user has sufficient balance to upgrade to the next tier.

  4. Event emission: Update the event emission to include the previous and current tier indices for better tracking.

Updated Code Snippet

function upgradeTier(address daoMembershipAddress, uint256 fromTierIndex) external {
// Input validation
require(fromTierIndex < daos[daoMembershipAddress].noOfTiers, "Invalid tier index.");
​
// Tier availability
require(daos[daoMembershipAddress].tiers[fromTierIndex + 1].minted < daos[daoMembershipAddress].tiers[fromTierIndex + 1].amount, "Next tier is full.");
​
// User balance
require(IERC20(daos[daoMembershipAddress].currency).balanceOf(_msgSender()) >= daos[daoMembershipAddress].tiers[fromTierIndex + 1].price, "Insufficient balance.");
​
// Burn the current tier token
IMembershipERC1155(daoMembershipAddress).burn(_msgSender(), fromTierIndex, 1);
​
// Mint the new tier token
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), fromTierIndex + 1, 1);
​
// Emit event with previous and current tier indices
emit UserUpgradedDAO(_msgSender(), daoMembershipAddress, fromTierIndex, fromTierIndex + 1);
}

Changes

  1. Added input validation for fromTierIndex.

  2. Verified that the next higher tier is available before upgrading the user.

  3. Checked if the user has sufficient balance to upgrade to the next tier.

  4. Updated the event emission to include the previous and current tier indices.

New Event

event UserUpgradedDAO(address indexed user, address indexed daoMembershipAddress, uint256 previousTierIndex, uint256 currentTierIndex);

This new event allows for better tracking of user tier upgrades, including the previous and current tier indices.

Updates

Lead Judging Commences

0xbrivan2 Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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