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

Underflow Issue in `viewWhitelistedCurrencies` due to Invalid Cursor Parameter

Fortis Audits

Summary

The function viewWhitelistedCurrencies in the CurrencyManager contract allows users to retrieve a paginated list of whitelisted currencies based on the cursor and size parameters. However, if the cursor provided by the user is greater than the total number of whitelisted currencies, an underflow will occur, causing the function to revert unexpectedly.

Vulnerability Details

  • In the viewWhitelistedCurrencies function, the length of the paginated response is calculated by checking if the requested size exceeds the available whitelisted currencies starting from the given cursor. If cursor is greater than or equal to _whitelistedCurrencies.length(), then _whitelistedCurrencies.length() - cursor results in an underflow, causing a revert.

  • This occurs because the subtraction is unsigned and cannot handle values where cursor > _whitelistedCurrencies.length(). Users providing a high cursor value, either accidentally or maliciously, will encounter unexpected reverts when attempting to call this function.

https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/dao/CurrencyManager.sol#L86-#L104

function viewWhitelistedCurrencies(
uint256 cursor,
uint256 size
) external view override returns (address[] memory, uint256) {
uint256 length = size;
​
@-> if (length > _whitelistedCurrencies.length() - cursor) {
length = _whitelistedCurrencies.length() - cursor;
}
. . .

Impact

This vulnerability can disrupt the user experience and potentially impact front-end interfaces that rely on this function to fetch paginated data. An attacker could also use this to intentionally trigger reverts, disrupting the UI or causing denial-of-service for users attempting to view whitelisted currencies.

Proof of Concept

  1. User calls the viewWhitelistedCurrencies function with a cursor value greater than by mistake this will cause the function to revert due to underflow.

  2. Due to this underflow, the function will revert and the user will not be able to retrieve the paginated list of whitelisted currencies.

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_FortisAudits_Underflow -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";
​
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 test_FortisAudits_Underflow() public {
vm.startPrank(admin);
for(uint i = 1; i < 10; i++) {
currencyManager.addCurrency(address(uint160(i)));
}
vm.stopPrank();
currencyManager.viewCountWhitelistedCurrencies();
vm.expectRevert();
currencyManager.viewWhitelistedCurrencies(11, 11);
}
}

Output of the test

VM::stopPrank()
│ └─ ← [Return]
├─ [390] CurrencyManager::viewCountWhitelistedCurrencies() [staticcall]
│ └─ ← [Return] 10
├─ [0] VM::expectRevert(custom error 0xf4844814)
│ └─ ← [Return]
├─ [544] CurrencyManager::viewWhitelistedCurrencies(11, 11) [staticcall]
│ └─ ← [Revert] panic: arithmetic underflow or overflow (0x11)
└─ ← [Stop]

Mitigation

To mitigate this, add a check at the beginning of the function to ensure that cursor is within bounds:

function viewWhitelistedCurrencies(
uint256 cursor,
uint256 size
) external view override returns (address[] memory, uint256) {
uint256 length = size;
​
+ require(cursor < _whitelistedCurrencies.length(), "Invalid cursor");
if (length > _whitelistedCurrencies.length() - cursor) {
length = _whitelistedCurrencies.length() - cursor;
}
...
}

This check prevents cursor from exceeding the length of _whitelistedCurrencies, avoiding the underflow and ensuring consistent and safe function execution.

Updates

Lead Judging Commences

0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!