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

sendProfit()::MembershipERC1155.sol could transfer tokens to contract instead of creator

0xziin

Summary

In case of minting 1 or multiple tokens with a token ID > or equal to 8, after using the function burnBatch() or burnBatchMultiple() to burn those tokens, sendProfit() will always transfer the tokens to the contract instead of the creator as it is supposed to.

https://github.com/Cyfrin/2024-11-one-world/blob/main/contracts/dao/tokens/MembershipERC1155.sol

Vulnerability Details

=> mint() authorizes minting NFTs with any ID number we want, there is no restriction.
(Note: Even if we could assume that it was supposed to mint only from ID=0 to ID=6 (7 IDs) in regard to the ShareOf() function that only takes in consideration from ID=0 to ID=6)

/// @notice Calculates the share of total profits for an account
/// @param account The account to query
/// @return The weighted share of the account
function shareOf(address account) public view returns (uint256) {
return (balanceOf(account, 0) * 64) +
(balanceOf(account, 1) * 32) +
(balanceOf(account, 2) * 16) +
(balanceOf(account, 3) * 8) +
(balanceOf(account, 4) * 4) +
(balanceOf(account, 5) * 2) +
balanceOf(account, 6);
}

=> burnBatch() and burnBatchMultiple() don't erase all the NFTs as they are supposed to do, only the 8th first IDs, from ID=0 to ID=7. Then, if more than ID number 7 are minted, TotalSupply will always be != 0 after using burnBatch() and burnBatchMultiple() because of the for loops, looping only from ID=0 to ID=7.

/// @notice Burn all tokens of a single user
/// @param from The address from which tokens will be burned
function burnBatch(address from) public onlyRole(OWP_FACTORY_ROLE) {
for (uint256 i = 0; i < 7; ++i) {
uint256 amount = balanceOf(from, i);
if (amount > 0) {
burn_(from, i, amount);
}
}
}
​
/// @notice Burn all tokens of multiple users
/// @param froms The addresses from which tokens will be burned
function burnBatchMultiple(address[] memory froms)
public
onlyRole(OWP_FACTORY_ROLE)
{
for(uint256 j = 0; j < froms.length; ++j){
for(uint256 i = 0; i < 7; ++i){
uint256 amount = balanceOf(froms[j], i);
if (amount > 0) {
burn_(froms[j], i, amount);
}
}
}
}

Then if ID(s) minted is/are > or equal to 8, all IDs > 7 will not be burned, resulting in a TotalSupply always > 0.

==> sendProfit() will always transfer tokens to the contract instead of the creator in case we mint tokens with IDs >= 8 and try to burn them using burnBatch() or burnBatchMultiple() afterward.
Tokens should be transfered to the creator but they won't be.

/// @notice Distributes profits to token holders
/// @param amount The amount of currency to distribute
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); // Redirect profit to creator if no supply
}
}

Impact

Tokens transfered to the wrong address when executing sendProfit() in those specific conditions.

Tools Used

Manual review, Github.

Recommendations

Cap the minting with a limit/maximum ID and use this limit ID for the loop in the burning process of burnBatch() and burnBatchMultiple().

function mint(address to, uint256 tokenId, uint256 amount) external override onlyRole(OWP_FACTORY_ROLE) {
require(tokenId <= limitID, "Can not mint ID > limitID");
totalSupply += amount * 2 ** (6 - tokenId); // Update total supply with weight
_mint(to, tokenId, amount, "");
}
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

Appeal created

0xziin Submitter
about 1 year ago
0xziin Submitter
about 1 year ago
0xbrivan2 Lead Judge
about 1 year ago
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!