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

Missing Max Token ID Check in URI Retrieval Leads to Access of Nonexistent URIs

theirrationalone

Summary

In the MembershipERC1155 contract, a public function called MembershipERC1155::uri allows anyone to retrieve the URI for any token ID. The protocol has a limited token supply with IDs ranging from 0 to 6, but if a user attempts to retrieve the URI for tokenId = 7, they can still obtain a response. To enhance security, it would be beneficial to enforce a maximum token ID check and, potentially, restrict URI retrieval access to only authorized minters.

MembershipERC1155::uri:

function uri(uint256 tokenId) public view virtual override returns (string memory) {
@> // Missing tokenId validation: tokenId should be < 7
// Missing user existence check: consider implementing if intended
return string(
abi.encodePacked(
super.uri(tokenId),
Strings.toHexString(uint256(uint160(address(this))), 20),
"/",
Strings.toString(tokenId)
)
);
}

Vulnerability Details

The following Foundry test demonstrates this vulnerability. Follow these steps to set up and run the test:

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

  2. Install all required dependencies.

  3. Configure necessary remappings.

  4. Create a file called MembershipERC1155Test.sol within the test folder.

  5. Add the following code in 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 testRetrievingUrisWithNonExistentTokenIds() public {
MembershipERC1155 daoMemberToken = baseFunction();
​
console.log("URI: ", daoMemberToken.uri(10));
}
}

  1. Create a mock folder in the test directory.

  2. Add CurrencyMock.sol and OWPWalletMock.sol within this folder, and implement the mock contracts as needed.

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

forge test --mt testRetrievingUrisWithNonExistentTokenIds -vv

  1. Review the test results:

[⠊] Compiling...
[⠘] Compiling 1 files with Solc 0.8.22
[⠃] Solc 0.8.22 finished in 3.65s
Compiler run successful!
​
Ran 1 test for test/MembershipERC1155Test.t.sol:MembershipERC1155Test
[PASS] testRetrievingUrisWithNonExistentTokenIds() (gas: 1532975)
Logs:
URI: https://one-world.com/uri/0xae36b27ba1fae8600e7c63ab822c62220109a63a/10
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.27ms (431.70µs CPU time)
​
Ran 1 test suite in 7.20ms (2.27ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

As shown, the uri function returns a URI even for a nonexistent token ID, which could mislead users.

Impact

  • Potential for user misinterpretation.

  • Unintentional behavior due to lack of token ID validation.

Tools Used

Manual Review, Foundry

Recommendations

Implement a check to prevent URI retrieval for tokenId > 6. Example:

function uri(uint256 tokenId) public view virtual override returns (string memory) {
+ require(tokenId < 7, "non-existent tokenId");
return string(
abi.encodePacked(
super.uri(tokenId),
Strings.toHexString(uint256(uint160(address(this))), 20),
"/",
Strings.toString(tokenId)
)
);
}

If restricting access to authorized minters is intended, consider the following:

function uri(uint256 tokenId) public view virtual override returns (string memory) {
+ require(tokenId < 7, "non-existent tokenId");
+ require(balanceOf(msg.sender, tokenId) > 0, "not allowed to see uri");
return string(
abi.encodePacked(
super.uri(tokenId),
Strings.toHexString(uint256(uint160(address(this))), 20),
"/",
Strings.toString(tokenId)
)
);
}
Updates

Lead Judging Commences

0xbrivan2 Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
0xbrivan2 Lead Judge 10 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.