Summary
MembershipERC1155::sendProfit reverts with overflow/underflow error if the amount sent is large.
Vulnerability Details
The vulnerability occurs in the calculation `(amount * ACCURACY) / _totalSupply` where:
`ACCURACY` is defined as 1e30 (a large constant for precision). Here's how it's defined in the code: `uint256 internal constant ACCURACY = 1e30;`
`amount` is an unbounded uint256 input parameter
`_totalSupply` is calculated based on weighted token balances
Function in question:
https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/dao/tokens/MembershipERC1155.sol#L191C1-L200C6
function sendProfit(uint256 amount) external {
uint256 _totalSupply = totalSupply;
if (_totalSupply > 0) {
totalProfit += (amount * ACCURACY) / _totalSupply;
IERC20(currency).safeTransferFrom(msg.sender, address(this), amount);
emit Profit(amount);
} else {
IERC20(currency).safeTransferFrom(msg.sender, creator, amount);
}
}
sendProfit function is an external function, callable by anyone with any amount. If the amount is too large, the function will revert with an overflow/underflow error becayse in order to calculate totalProfit, it multiplies the `amount` with the `ACCURACY`, that is itself a very large number of 1e30.
POC
Add Foundry to the project, inside the test folder, create a folder named foundry, and inside of that folder create a new file named SendProfitOverflow.t.sol. Paste the following test suite into the file. This test suite, after initiating the required protocol state, will create a new sponsored DAO with the same values that are present in the protocol team's test file MembershipFactory.test.ts. There are 2 users, Alice and Bob who join to the DAO, and then DAOCreator (could be anyone) sends some amount of profit to the function.
pragma solidity 0.8.22;
import {Test, console} from "forge-std/Test.sol";
import {MembershipERC1155} from "../../contracts/dao/tokens/MembershipERC1155.sol";
import {MembershipFactory} from "../../contracts/dao/MembershipFactory.sol";
import {CurrencyManager} from "../../contracts/dao/CurrencyManager.sol";
import {OWPERC20} from "../../contracts/shared/testERC20.sol";
import {DAOInputConfig, DAOType, TierConfig} from "../../contracts/dao/libraries/MembershipDAOStructs.sol";
import "forge-std/Test.sol";
interface IERC1155 {
function balanceOf(address account, uint256 id) external view returns (uint256);
}
contract SendProfitOverflowTest is Test {
CurrencyManager public currencyManager;
MembershipFactory public membershipFactory;
OWPERC20 public testERC20;
address Alice = makeAddr("Alice");
address Bob = makeAddr("Bob");
address DAOCreator = makeAddr("DAOCreator");
address oneWorldPlatformWalletAddress = makeAddr("oneWorldPlatformWalletAddress");
MembershipERC1155 public membershipERC1155Implementation;
uint256 maxAmount = type(uint256).max;
function setUp() public {
currencyManager = new CurrencyManager();
testERC20 = new OWPERC20("Test Token", "TST");
membershipERC1155Implementation = new MembershipERC1155();
membershipFactory = new MembershipFactory(
address(currencyManager),
address(oneWorldPlatformWalletAddress),
"https://baseURI/",
address(membershipERC1155Implementation)
);
currencyManager.addCurrency(address(testERC20));
deal(address(testERC20), DAOCreator, maxAmount);
deal(address(testERC20), Alice, maxAmount);
deal(address(testERC20), Bob, maxAmount);
}
function test_profit_sharing_sponsored_dao(uint256 _profitAmount) public {
vm.startPrank(DAOCreator);
DAOInputConfig memory daoConfig = DAOInputConfig({
ensname: "testdao.eth",
daoType: DAOType.SPONSORED,
currency: address(testERC20),
maxMembers: 1270,
noOfTiers: 7
});
TierConfig[] memory tierConfigs = new TierConfig[]();
tierConfigs[0] = TierConfig({amount: 640, price: 6400, power: 64, minted: 0});
tierConfigs[1] = TierConfig({amount: 320, price: 3200, power: 32, minted: 0});
tierConfigs[2] = TierConfig({amount: 160, price: 1600, power: 16, minted: 0});
tierConfigs[3] = TierConfig({amount: 80, price: 800, power: 8, minted: 0});
tierConfigs[4] = TierConfig({amount: 40, price: 400, power: 4, minted: 0});
tierConfigs[5] = TierConfig({amount: 20, price: 200, power: 2, minted: 0});
tierConfigs[6] = TierConfig({amount: 10, price: 100, power: 1, minted: 0});
address newDAOAddress = membershipFactory.createNewDAOMembership(daoConfig, tierConfigs);
console.log(newDAOAddress);
vm.stopPrank();
vm.startPrank(Alice);
testERC20.approve(address(membershipFactory), maxAmount);
membershipFactory.joinDAO(newDAOAddress, 0);
IERC1155 deployedDAO = IERC1155(newDAOAddress);
console.log("Alice's 1155 tokens", deployedDAO.balanceOf(Alice, 0));
vm.stopPrank();
vm.startPrank(Bob);
testERC20.approve(address(membershipFactory), maxAmount);
membershipFactory.joinDAO(newDAOAddress, 6);
console.log("Bob's 1155 tokens", deployedDAO.balanceOf(Bob, 6));
vm.stopPrank();
vm.startPrank(DAOCreator);
testERC20.approve(address(newDAOAddress), maxAmount);
MembershipERC1155(newDAOAddress).sendProfit(_profitAmount);
vm.stopPrank();
uint256 AliceShare = MembershipERC1155(newDAOAddress).shareOf(Alice);
uint256 BobShare = MembershipERC1155(newDAOAddress).shareOf(Bob);
console.log("Alice's share", AliceShare);
console.log("Bob's share", BobShare);
}
}
Now, on the terminal, run forge test -vvvv --match-contract SendProfitOverflowTest
The test will revert with an output similar to this one:
... shortened for readibility
├─ [0] VM::startPrank(DAOCreator: [0x60F288AEb2240e72083BF614cb338a09768cC50B])
│ └─ ← [Return]
├─ [24739] OWPERC20::approve(TransparentUpgradeableProxy: [0xDD4c722d1614128933d6DC7EFA50A6913e804E12], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77])
│ ├─ emit Approval(owner: DAOCreator: [0x60F288AEb2240e72083BF614cb338a09768cC50B], spender: TransparentUpgradeableProxy: [0xDD4c722d1614128933d6DC7EFA50A6913e804E12], value: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77])
│ └─ ← [Return] true
├─ [995] TransparentUpgradeableProxy::sendProfit(115792089237316195423570985008687907853269984665640564039457584007913129639933 [1.157e77])
│ ├─ [543] MembershipERC1155::sendProfit(115792089237316195423570985008687907853269984665640564039457584007913129639933 [1.157e77]) [delegatecall]
│ │ └─ ← [Revert] panic: arithmetic underflow or overflow (0x11)
│ └─ ← [Revert] panic: arithmetic underflow or overflow (0x11)
└─ ← [Revert] panic: arithmetic underflow or overflow (0x11)
Suite result: FAILED. 1 passed; 1 failed; 0 skipped; finished in 6.98ms (3.85ms CPU time)
Ran 1 test suite in 1.10s (6.98ms CPU time): 1 tests passed, 1 failed, 0 skipped (2 total tests)
Failing tests:
Encountered 1 failing test in test/foundry/SendProfitOverflow.t.sol:SendProfitOverflowTest
[FAIL: panic: arithmetic underflow or overflow (0x11); counterexample: calldata=0x7d1a3bbefffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffd args=[115792089237316195423570985008687907853269984665640564039457584007913129639933 [1.157e77]]] test_profit_sharing_sponsored_dao(uint256) (runs: 2, μ: 1903086, ~: 1903086)
Impact
High/Medium. Likelihood of this situation occuring is medium, but the impact is high. Large amounts will never be able to be sent to the function.
- The overflow causes the profit distribution system to completely fail
- This is a severe disruption of core protocol functionality (profit sharing)
- It completely breaks a core protocol functionality (profit distribution)
- While it requires specific conditions, these conditions are quite feasible in a real-world scenario
- It affects all users of the DAO simultaneously
- The fix is critical for the protocol to function as intended
- There's no workaround once the issue occurs - the transaction will always revert
Tools Used
Foundry
Recommendations
Consider implementing a different profit-sharing mechanism that:
Uses smaller precision numbers (e.g., 1e18 instead of 1e30)
Implements batch processing for large amounts
