From 8431c80cce81afaaacf3a3f04a62b80a6344b93a Mon Sep 17 00:00:00 2001 From: codingsh Date: Tue, 14 Mar 2023 10:55:51 +0000 Subject: [PATCH] chore(contracts): fixed critical security issues --- contracts/protocol/Sector3DAO.sol | 24 ++++++++------ contracts/protocol/Sector3DAOFactory.sol | 14 ++++++-- contracts/protocol/Sector3DAOPriority.sol | 39 +++++++++++++++++++++-- 3 files changed, 63 insertions(+), 14 deletions(-) diff --git a/contracts/protocol/Sector3DAO.sol b/contracts/protocol/Sector3DAO.sol index 7d70eb7..8d02445 100644 --- a/contracts/protocol/Sector3DAO.sol +++ b/contracts/protocol/Sector3DAO.sol @@ -46,32 +46,34 @@ contract Sector3DAO { owner = msg.sender; } + modifier onlyOwner() { + require(msg.sender == owner, "You are not authorized to perform this action."); + _; + } + + /** * Updates the DAO's name. */ - function setName(string calldata name_) public { - require(msg.sender == owner, "You aren't the owner"); + function setName(string calldata name_) public onlyOwner { name = name_; } /** * Updates the DAO's purpose. */ - function setPurpose(string calldata purpose_) public { - require(msg.sender == owner, "You aren't the owner"); + function setPurpose(string calldata purpose_) public onlyOwner { purpose = purpose_; } /** * Updates the DAO's token. */ - function setToken(address token_) public { - require(msg.sender == owner, "You aren't the owner"); + function setToken(address token_) public onlyOwner { token = token_; } - function deployPriority(string calldata title, address rewardToken, uint16 epochDurationInDays, uint256 epochBudget) public returns (Sector3DAOPriority) { - require(msg.sender == owner, "You aren't the owner"); + function deployPriority(string calldata title, address rewardToken, uint16 epochDurationInDays, uint256 epochBudget) public onlyOwner returns (Sector3DAOPriority) { Sector3DAOPriority priority = new Sector3DAOPriority(address(this), title, rewardToken, epochDurationInDays, epochBudget); priorities.push(priority); return priority; @@ -85,8 +87,8 @@ contract Sector3DAO { return priorities; } - function removePriority(Sector3DAOPriority priority) public { - require(msg.sender == owner, "You aren't the owner"); + function removePriority(Sector3DAOPriority priority) public onlyOwner { + require(!priority.isInVotingPeriod(), "Cannot remove priority during voting period"); Sector3DAOPriority[] memory prioritiesAfterRemoval = new Sector3DAOPriority[](priorities.length - 1); uint16 prioritiesIndex = 0; for (uint16 i = 0; i < prioritiesAfterRemoval.length; i++) { @@ -98,4 +100,6 @@ contract Sector3DAO { } priorities = prioritiesAfterRemoval; } + + } diff --git a/contracts/protocol/Sector3DAOFactory.sol b/contracts/protocol/Sector3DAOFactory.sol index 9aa4c06..1c87e13 100644 --- a/contracts/protocol/Sector3DAOFactory.sol +++ b/contracts/protocol/Sector3DAOFactory.sol @@ -9,6 +9,8 @@ contract Sector3DAOFactory { address[] public daos; + error YouAreNotAuthorized(); + constructor() { owner = msg.sender; // daos.push(0x5FbDB2315678afecb367f032d93F642f64180aa3); // localhost @@ -18,7 +20,15 @@ contract Sector3DAOFactory { daos.push(0x9741B82017485759c9Bcc13FeA10c1105f82d25C); // Bankless Africa (v0) } - function setOwner(address owner_) public { + + + modifier onlyOwner() { + require(msg.sender == owner, "You are not authorized to perform this action."); + _; +} + + + function setOwner(address owner_) public onlyOwner { require(msg.sender == owner, "You aren't the owner"); owner = owner_; } @@ -33,7 +43,7 @@ contract Sector3DAOFactory { return address(dao); } - function removeDAO(address dao) public { + function removeDAO(address dao) public onlyOwner { require(msg.sender == owner, "You aren't the owner"); address[] memory daosAfterRemoval = new address[](daos.length - 1); uint16 daosIndex = 0; diff --git a/contracts/protocol/Sector3DAOPriority.sol b/contracts/protocol/Sector3DAOPriority.sol index 4f03983..137fc2e 100644 --- a/contracts/protocol/Sector3DAOPriority.sol +++ b/contracts/protocol/Sector3DAOPriority.sol @@ -90,9 +90,19 @@ contract Sector3DAOPriority is IPriority { if (epochReward == 0) { revert NoRewardForEpoch(); } - rewardToken.transfer(msg.sender, epochReward); + bool epochFunded = isEpochFunded(epochIndex); + if (!epochFunded) { + revert EpochNotYetFunded(); + } + bool rewardClaimed = isRewardClaimed(epochIndex, msg.sender); + if (rewardClaimed) { + revert RewardAlreadyClaimed(); + } + claims[epochIndex][msg.sender] = true; + claimsBalance += epochReward; + require(rewardToken.transfer(msg.sender, epochReward), "Reward transfer failed"); emit RewardClaimed(epochIndex, msg.sender, epochReward); - } +} /** * Calculates a contributor's token allocation of the budget for a given epoch. @@ -104,6 +114,31 @@ contract Sector3DAOPriority is IPriority { return epochBudget * allocationPercentage / 100; } + /** + * @notice Checks if the smart contract has received enough funding to cover claims for a past epoch. + * + * @param epochIndex The index of the epoch to check. + * @return A boolean indicating whether the epoch is funded or not. + * + * @dev This function loops through all past epochs to calculate whether the current epoch is funded or not. + * If the number of past epochs becomes very large, the function may consume an excessive amount of gas and fail to execute, + * thereby preventing other legitimate functions from executing. Epochs without contributions are excluded from funding. + */ + + function isEpochFunded(uint16 epochIndex) public view returns (bool) { + if (epochIndex >= getEpochIndex()) { + revert EpochNotYetEnded(); + } + if (getEpochContributions(epochIndex).length == 0) { + return false; + } + uint256 totalBudget = epochBudget * (epochIndex + 1); + uint256 totalFundingReceived = rewardToken.balanceOf(address(this)) + claimsBalance; + uint16 numberOfEpochsWithContributions = epochIndex + 1; + return totalFundingReceived >= totalBudget; + } + + /** * Calculates a contributor's percentage allocation of the budget for a given epoch. *