Summary
The getUnsaved function in the MembershipERC1155 contract creates precision loss when calculating the profit share for each account due to the handling of large profit values and the use of integer division. This precision loss affects the accuracy of profit distribution, potentially undercompensating users with low profit balances and creating discrepancies between expected and actual distributed profits.
Vulnerability Details
The precision loss issue originates from the getUnsaved function, which calculates unsaved profits for an account by dividing the difference between totalProfit and lastProfit by a fixed ACCURACY constant (1e30). Since Solidity only supports integer division, any remainder from the division operation is truncated, resulting in profit values that may be lower than intended. This affects users' balances, as they may not receive the exact share of profits.
https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/dao/tokens/MembershipERC1155.sol#L162-#L164
function getUnsaved(address account) internal view returns (uint256 profit) {
@-> return ((totalProfit - lastProfit[account]) * shareOf(account)) / ACCURACY;
}
The division by ACCURACY introduces a precision loss, especially when shareOf(account) yields a small value relative to the overall totalProfit. This leads to truncated profit values and affects the distribution accuracy, particularly for accounts with lower profit shares.
Impact
The precision loss impacts profit distribution accuracy for users, especially those with low balances. This could result in:
Users receiving less profit than they are entitled to, particularly if totalProfit is a large number relative to individual shares.
An unfair distribution of profits, where smaller holders or accounts with lower profit allocations lose a portion of their claim.
This precision loss can accumulate over multiple profit distributions, leading to significant discrepancies over time.
Proof of Concept
Alice creates a DAO membership contract using the MembershipFactory contract.
Bluedragon buys a membership token of tierIndex 6 from the DAO contract.
Purpledragon buys a membership token of tierIndex 2 from the DAO contract.
Alice sends profit of 100e6 USDC to the DAO contract.
Bluedragon and Purpledragon claim their profits using the claimProfit function.
The getUnsaved function calculates the profit share for each account, but due to precision loss, the profit distribution is inaccurate.
Manual Calculations
MemberShipERC1155.sol
Factory calls mint() function once user purchases a tier of the membership
=> Mint(to: Bluedragon, tokenid: 6, amount: 1)
totalSupply = totalSupply + amount * 2 ** (6-tokenid)
totalSupply = 0 + 1 * 2 ** (6-6) = 1 * 2 ^ 0 = 1
_mint(bludragon, 6, 1)
balanceOf[bluedragon][6] = 1
*-----------------------------------------------------------------*
Factory mints purpledragon with tierIndex 2
=> mint(to: purpledragon, tokenId: 2)
totalSupply = totalSupply + amount * 2 & (6-tokenid)
totalSupply = 1 + 1 * 2 ^ (6-2) = 1 + 1 * 2 ^ 4 = 17
_mint(bludragon, 2, 1)
balanceOf[purpledragon][2] = 1
*-----------------------------------------------------------------*
Alice calls sendProfit(amount)
=> sendProfit(100e6)
_totalSupply = 17
If(_totalSupply > 1) true {
totalProfit += (amount * ACCURACY(1e30))/ _totalSupply
= 0 + (100e6 * 1e30) / 17 = 5882352941176470588235294117647058823
SafeTransferFrom(alice, address(this), 100e6)
}
*-----------------------------------------------------------------*
Purpledragon Calls profitOf(account) to check profit
=> profitOf(purpledragon )
savedProfit[purpledragon] + getUnsaved(purpledragon )
getUnsaved(purpledragon) is called
=>
((totalProfit - lastProfit[account]) * shareOf(account)) / ACCURACY;
= ((5882352941176470588235294117647058823 - 0) x shareOf(purpledragon)) / 1e30
ShareOf(purpledragon ) called
=>
(balanceOf(account, 0) x 64) +
(balanceOf(account, 1) x 32) +
(balanceOf(account, 2) x 16) +
(balanceOf(account, 3) x 8) +
(balanceOf(account, 4) x 4) +
(balanceOf(account, 5) x 2) +
balanceOf(account, 6);
0+0+1*16+0+0+0+0 = 16
= (5882352941176470588235294117647058823 x 16) / 1e30
= 94117647.058823529411764705882352941168
= 94_117_647
= 0 + 94.117_647 USDC
purpledragon gets 94_117_647 USDC
*-----------------------------------------------------------------*
Purpledragon calls claimProfit()
=>
Profit = saveProfit(bluedragon)
saveProfit() called
=>
Unsaved = getUnsaved(bluedragon)
Unsaved = 94_117_647
lastProfit[purpledragon] = 94_117_647
Profit = savedProfit[purpledragon] + 94_117_647
Profit = 0 + 94_117_647 = 94_117_647
savedProfit[purpledragon] = 94_117_647
Profit = 94_117_647
Profit > 0 true
savedProfit[purpledragon] = 0
safeTransfer(prupledragon, 94_117_647)
prupledragon receives 94_117_647 usdc
*-----------------------------------------------------------------*
Bluedragon calls claimProfit()
=>
Profit = saveProfit(bluedragon)
saveProfit() called
=>
Unsaved = getUnsaved(bluedragon)
Unsaved = 100e6
=>
getUnsaved(bluedragon) is called
=>
((totalProfit - lastProfit[account]) * shareOf(account)) / ACCURACY;
= ((5882352941176470588235294117647058823 - 0) x shareOf(bluedragon)) / 1e30
ShareOf(bluedragon ) called
=>
(balanceOf(account, 0) x 64) +
(balanceOf(account, 1) x 32) +
(balanceOf(account, 2) x 16) +
(balanceOf(account, 3) x 8) +
(balanceOf(account, 4) x 4) +
(balanceOf(account, 5) x 2) +
balanceOf(account, 6);
0+0+0+0+0+0+1 = 1
= (5882352941176470588235294117647058823 x 1) / 1e30
= 5882352.941176470588235294117647058823
= 5_882_352
= 0 + 5.882_352 USDC
bluedragon gets 5_882_352 USDC
lastProfit[bluedragon]=5882352941176470588235294117647058823
Profit = savedProfit[bluedragon] + 5_882_352
Profit = 0 + 5_882_352 = 5_882_352
savedProfit[bluedragon] = 5_882_3526
Profit = 5_882_352e6
Profit > 0 true
savedProfit[bluedragon] = 0
safeTransfer(bluedragon, 5_882_352e6)
Bluedragon receives 5.882_352 usdc
send Amount is 100_000_000
claimed amount is 5_882_352 + 94_117_647 = 99999999
Boom!!! Precision Loss
Proof Of Code
Create a BugTest.t.sol contract in the test folder.
Add the following code to the BugTest.t.sol file:
Run the test using the command forge test --mt test_FortisAudits_PrecisionLoss -vvvv.
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 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_FortisAudits_PrecisionLoss() public {
usdc.mint(bluedragon, 100e6);
usdc.mint(alice, 100e6);
usdc.mint(purpledragon, 100e6);
DAOInputConfig memory config;
TierConfig[] memory tiers;
(config, tiers) = Inputs();
vm.startPrank(alice);
usdc.approve(address(membershipFactory), type(uint256).max);
address new_DAO_addr = membershipFactory.createNewDAOMembership(config, tiers);
vm.stopPrank();
MembershipERC1155 new_DAO = MembershipERC1155(new_DAO_addr);
vm.startPrank(bluedragon);
usdc.approve(address(membershipFactory), type(uint256).max);
membershipFactory.joinDAO(new_DAO_addr, 6);
vm.stopPrank();
vm.startPrank(purpledragon);
usdc.approve(address(membershipFactory), type(uint256).max);
membershipFactory.joinDAO(new_DAO_addr, 2);
vm.stopPrank();
vm.startPrank(alice);
usdc.approve(new_DAO_addr, type(uint256).max);
new_DAO.sendProfit(100e6);
vm.stopPrank();
console.log("*--------------After sendProfit--------------*");
console.log("Balance of DAO including joinDAO funds: ", usdc.balanceOf(new_DAO_addr));
new_DAO.totalSupply();
vm.prank(bluedragon);
uint256 profit_bluedragon = new_DAO.claimProfit();
vm.prank(purpledragon);
uint256 profit_purpledragon = new_DAO.claimProfit();
uint256 Amount = tiers[6].price + tiers[2].price;
uint256 fee = (Amount * 20) / 100;
uint256 paidAmount = Amount - fee;
console.log("*--------------After claimProfit--------------*");
console.log("Profit of Bluedragon: ", profit_bluedragon);
console.log("Profit of Purpledragon: ", profit_purpledragon);
console.log("Balance of DAO After Precision Loss: ", usdc.balanceOf(new_DAO_addr) - paidAmount);
assertGt(usdc.balanceOf(new_DAO_addr) - paidAmount, 0);
}
}
Output of the test
[PASS] test_FortisAudits_PrecisionLoss() (gas: 2015414)
Logs:
*--------------After sendProfit--------------*
Balance of DAO including joinDAO funds: 116000000
*--------------After claimProfit--------------*
Profit of Bluedragon: 5882352
Profit of Purpledragon: 94117647
Balance of DAO After Precision Loss: 1
Mitigation
To mitigate this precision loss, consider using a more precise method for calculating profit shares and distributions.
Here is the recommended mitigation:
- function getUnsaved(address account) internal view returns (uint256 profit) {
- return ((totalProfit - lastProfit[account]) * shareOf(account)) / ACCURACY;
- }
+ function getUnsaved(address account) internal view returns (uint256 profit) {
+ uint256 profitShare = (totalProfit - lastProfit[account]) * shareOf(account);
+ profit = (profitShare + ACCURACY / 2) / ACCURACY;
+
+ }
