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

Missing Access Control in MembershipERC1155.sendProfit()

0bingo76

Summary

The MembershipERC1155 contract inherits from AccessControlUpgradeable but fails to implement access control on the critical sendProfit function. This oversight allows any external actor to manipulate the profit distribution system.

https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/dao/tokens/MembershipERC1155.sol#L191-L200

Vulnerability Details

The contract implements AccessControlUpgradeable and defines roles:

contract MembershipERC1155 is ERC1155Upgradeable, AccessControlUpgradeable {
bytes32 public constant OWP_FACTORY_ROLE = keccak256("OWP_FACTORY_ROLE");
bytes32 public constant DAO_CREATOR = keccak256("DAO_CREATOR");
// Critical function missing access control
function sendProfit(uint256 amount) external {
uint256 _totalSupply = totalSupply;
if (_totalSupply > 0) {
totalProfit += (amount * ACCURACY) / _totalSupply; // Direct state manipulation
IERC20(currency).safeTransferFrom(msg.sender, address(this), amount);
emit Profit(amount);
}
}
}

Attack Vectors

  1. Profit Manipulation Attack

// Attack contract
contract ProfitManipulator {
MembershipERC1155 target;
function attack(uint256 amount) external {
// No role checks prevent this call
target.sendProfit(amount);
// totalProfit is now manipulated: (amount * 1e30) / totalSupply
}
}

  1. Supply-Based Attack

    • Monitor for low totalSupply periods

    • Exploit the division in (amount * ACCURACY) / _totalSupply

    • Maximize profit manipulation impact

Impact

  • Direct manipulation of profit calculations

  • Unauthorized redistribution of profits

  • Compromises the entire profit-sharing mechanism

Proof of Impact

// Example impact calculation
Initial state:
totalSupply = 1000 tokens
totalProfit = 100 ether
​
Attack:
amount = 10 ether
New totalProfit = 100 ether + (10 ether * 1e30) / 1000
// Results in massive profit inflation

Tools Used

  1. SolidityScan

    • Severity: Critical

    • Confidence: High

    • Finding: "The contract is importing an access control library but the function is missing the modifier"

  2. Manual Code Review

    • Pattern matching with known vulnerabilities (e.g., Perennial's Issue H-5)

    • Control flow analysis

    • Access control verification

Recommendations

Immediate Fix

contract MembershipERC1155 is ERC1155Upgradeable, AccessControlUpgradeable {
bytes32 public constant PROFIT_DISTRIBUTOR_ROLE = keccak256("PROFIT_DISTRIBUTOR_ROLE");
function sendProfit(uint256 amount) external onlyRole(PROFIT_DISTRIBUTOR_ROLE) {
require(amount > 0 && amount <= maxProfitAmount, "Invalid profit amount");
require(block.timestamp >= lastProfitTime + minProfitInterval, "Too frequent");
uint256 _totalSupply = totalSupply;
require(_totalSupply >= minTotalSupply, "Supply too low");
if (_totalSupply > 0) {
totalProfit += (amount * ACCURACY) / _totalSupply;
IERC20(currency).safeTransferFrom(msg.sender, address(this), amount);
emit Profit(amount);
lastProfitTime = block.timestamp;
}
}
}
Updates

Lead Judging Commences

0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice
0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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

Give us feedback!