ReBakedDAO

Smart Contract Audit Report

Audit Summary

ReBakedDAO Audit Report ReBaked is creating a new platform in which users can create their own Projects which can be used for budgeting and managing Collaborators.

For this audit, we reviewed the ReBakedDAO contract and supporting contracts provided to us by the project team.

Audit Findings

High findings were identified and the team must resolve these issues. In addition, centralized aspects are present.
Date: June 14th, 2022.
Updated: August 4th, 2022 to reflect changes made by the project team.

Finding #1 - ReBakedDAO - High (Resolved)

Description: The createPackages(), getMgps(), & getBonuses() functions each have multiple issues that would cause them to either fail or allow exploits.
Risk/Impact: Users will either not be able to call these functions without them failing or will be able to use them to drain the contract's funds.
Recommendation: As the team has stated that these functions are not intended to be used within the Project, they should be removed. Even if they are not intended to be used, they need to be removed as they can be used by attackers to easily exploit the contract.
Resolution: The team has removed these functions.

Finding #2 - ReBakedDAO - High (Resolved)

Description: If a Disputed Collaborator is approved, they will be able to withdraw their bonus multiple times.
Risk/Impact: An approved Disputed Collaborator can withdraw all of a Project's specified token from the contract.
Recommendation: The team should implement the following code to the beginning of the getBonus() function:
require(collaboratorData[projectId_][packageId_][collaborator_].timeBonusPaid == 0, "Bonus already paid");
Resolution: This function now correctly ensures that a collaborator has not already been paid.

Finding #3 - ReBakedDAO - High (Resolved)

Description: In the removeObserver() function, either of the following conditional blocks can be executed at any time to remove an Observer, as packageStatus is a parameter specified by the caller.
if(packageStatus[i] == true){
   payObserverFee(projectId_,packageId_[i],observer_);
   observerData[projectId_][packageId_[i]][observer_].isRemoved = true;
} else{
   observerData[projectId_][packageId_[i]][observer_].timeCreated = 0;
   packageData[projectId_][packageId_[i]].totalObservers -= 1;
   observerData[projectId_][packageId_[i]][observer_].isRemoved = true;
}
Risk/Impact: Scenario - packageStatus[i] == true: If an Observer is paid, then another Observer is added, the calculation for the paid Observer becomes incorrect, resulting in more than the allocated Observer funds to be distributed. As no further Observers can be added after the package is finished, this will not result in any issues if it can only be called in this case.
Scenario - packageStatus[i] == false: If an Observer is removed after the package is finished, some Observers may already have been paid, which will result in less funds distributed than originally allocated.
Recommendation: The first conditional block should only be permitted to be executed if the Package has been finished. The second conditional block should only be permitted to be called if the package has not been finished.
Resolution: A variation of the above recommendation has been implemented. If an Observer is removed before a Package is finished, the Package's number of Observers is decremented and the Observer is not paid. If the Package is finished, the number of Observers is not decremented and the Observer is paid.

Finding #4 - ReBakedDAO - High

Description: In the createPackage() function, the budgetWithDeduction_ parameter is used as the amount added to the Project's allocated budget.
Risk/Impact: Users can create a Package with a large budget and input a small value to be added to the allocated budget. This will give the user the ability to spend more funds than are allocated to them.
Recommendation: The budgetWithDeduction_ parameter should be removed, and the amount to add to the Project's allocated budget should instead be calculated using the other parameters.
Resolution: The team has not yet addressed this issue.

Finding #5 - ReBakedDAO - High

Description: In the canclePackage() function, the toBeReverted parameter is used as the amount decremented from the Project's allocated budget.
Risk/Impact: A Project creator can be allocated more funds to spend than they originally transferred to the contract by passing in a large toBeReverted value.
Recommendation: The toBeReverted parameter should be removed, and the amount to be decremented from the Project's allocated budget should instead be calculated. Alternatively, this function could be removed.
Update: A Project's allocated budget is now decremented by the Package's paid funds, which is incorrect. The Project's allocated funds should instead be decremented by the Package's unused funds.

Finding #6 - ReBakedDAO - High (Resolved)

Description: Observer fees are calculated by the total Observer budget divided by the total number of Observers. Observer Fee distributions will be incorrect if an Observer is added or removed after any other Observer has received payment.
Risk/Impact: An incorrect amount of funds could be distributed to Observers. This amount could be smaller or larger than the intended amount.
Exploit Scenario:
  1. One Observer exists. They collect their payment, which is calculated as the entire Observer budget.
  2. Another Observer is added and they collect their fee.
  3. This Observer's payment is calculated as 50% of the Observer budget. 150% of the Observer budget has been spent at this point.
Recommendation: The payObserverFee() function should only be permitted to be called once the Package is finished.
Resolution: The payObserverFee() function can now only be called once a Package is finished.

Finding #7 - ReBakedDAO - High (Resolved)

Description: The following code in the payObserverFee() function does not follow Checks-Effects-Interactions:
IERC20(projectData[projectId_].token).safeTransfer(observer_, concernedFee*10e17);
observerData[projectId_][packageId_][observer_].timePaid = block.timestamp;
observerData[projectId_][packageId_][observer_].isFeePaid = true;
Risk/Impact: If a user calls getObserverFee() upon receiving the tokens from the payObserverFee() function, they will be able to receive their payment twice.
Recommendation: The safeTransfer() call should occur after the state variables are updated to avoid reentrancy attacks.
Resolution: The team has implemented the above recommendation.

Finding #8 - ReBakedDAO - High (Resolved)

Description: The following code in the payMgp() function does not follow Checks-Effects-Interactions:
IERC20(projectData[projectId_].token).safeTransfer(collaborator_, amount_*10e17);
projectData[projectId_].budgetPaid += amount_;
packageData[projectId_][packageId_].budgetPaid += amount_;
collaboratorData[projectId_][packageId_][collaborator_].timeMgpPaid = block.timestamp;
Risk/Impact: If a user calls getMgp() upon receiving the tokens from the payMgp() function, they will be able to receive their payment twice.
Recommendation: The safeTransfer() call should occur after the state variables are updated to avoid Reentrancy attacks.
Resolution: The team has implemented the above recommendation.

Finding #9 - ReBakedDAO - High (Resolved)

Description: The following code in the rejectPayment() function does not follow Checks-Effects-Interactions:
IERC20(_token).safeTransfer(_initiator,_feesToBeRevert*10e17);
collaboratorData[_projectId][_packageId][_collaborator].mgp = 0;
collaboratorData[_projectId][_packageId][_collaborator].bonusScore = 0;
collaboratorData[_projectId][_packageId][_collaborator].isDisputeRaised = true;
collaboratorData[_projectId][_packageId][_collaborator].isMGPPaid = false;
collaboratorData[_projectId][_packageId][_collaborator].isBonusPaid = false;		
_downDispute(_projectId,_collaborator);
Risk/Impact: The Project creator can potentially reenter the contract once safeTransfer() is called and pay the Collaborator their MGP before it is reset.
Recommendation: The safeTransfer() call should not occur until the end of the rejectPayment() function.
Resolution: The team has implemented the above recommendation.

Finding #10 - ReBakedDAO - High (Resolved)

Description: The getMgp() and getBonus() functions do not properly check if a user has already received their MGP and bonus.
Risk/Impact: A user can claim their bonus, have a dispute raised and approved against them, then claim again for double rewards. A user could also potentially reenter if transferred funds in the payMgp() function and call getMgp() before their timeMgpPaid is set. This also exists in the payObserverFee() and getObserverFee() functions.
Recommendation: The following line should be added to the getMgp() function to ensure a user does not claim their MGP twice. A similar line should be added to the getBonus() function for the same purpose:
require(!collaboratorData[projectId_][packageId_][collaborator_].isMGPPaid,"MGP Paid");
Resolution: The team has implemented the above recommendation.

Finding #11 - ReBakedDAO - High

Description: Observers and Collaborators can be removed after they have already been paid.
Risk/Impact: If all Observers or Collaborators are removed after they have been paid, the Package's Observer Budget or Bonus Budget will be added to the Packages "unspent" funds when the Package is finished and will be incorrectly subtracted from the Project's total allocated budget, allowing the initiator to withdraw more than intended once the Project is finished.
Recommendation: Observers should not be permitted to be removed if they have already been paid their fee, and Collaborators should not be permitted to be removed if they have been paid their bonus.
Update: While Observers can no longer be removed if they have been paid, the issue persists when removing Collaborators.

Finding #12 - ReBakedDAO - High

Description: Removing a Collaborator with an unpaid MGP or bonus does not decrement the Package's allocated budget or bonus budget, respectively.
Risk/Impact: The Package will incorrectly lose the Collaborator's allocated funds if a Collaborator is removed with an unpaid bonus.
Recommendation: The Package's budget should be properly updated when a Collaborator is removed.
Update: Removing a Collaborator will now decrement the Package's allocated budget and bonus budget regardless of whether they have been paid or not. The budget and bonus budget should only be decremented if the Collaborator has been paid.

Finding #13 - ReBakedDAO - High

Description: If the addCollaborator() function is called on a Collaborator with an existing MGP, the old MGP is not subtracted from the Package's allocated budget after adding the new MGP.
Risk/Impact: The Package's allocated budget will be increased to a larger amount than intended if an existing Collaborator's MGP is updated, resulting in the user being unable to spend all their originally allocated funds for the Package.
Recommendation: If a user has an existing MGP, it should be deducted from the Package's allocated budget after being incremented by the new budget.
Resolution: The team has not yet addressed this issue.

Finding #14 - ReBakedDAO - High

Description: If the setBonusScores() function is called on a Collaborator with an existing bonus score, the old bonus score is not subtracted from the Package's allocated bonus budget after adding the new bonus score.
Risk/Impact: The Package's allocated bonus budget will be increased to a larger amount than intended if an existing Collaborator's bonus is updated, resulting in the user being unable to spend all their bonus funds allocated for the Package.
Recommendation: If a user has an existing bonus, it should be deducted from the Package's allocated bonus budget after being incremented by the new budget.
Resolution: The team has not yet addressed this issue.

Finding #15 - ReBakedDAO - High

Description: The setBonusScores() function does not check if the bonus amount exceeds the Package's bonus budget.
Risk/Impact: Bonuses can be granted for a Package that may not have enough bonus funds allocated.
Recommendation: The setBonusScores() function should ensure that added bonuses do not result in the Package exceeding its bonus budget.
Update: Only the scores added are compared to the Package's bonus budget. As a result the owner can add bonus scores multiple times in order to bypass this check.

Finding #16 - ReBakedDAO - High

Description: A Project creator can pay the Observer Fee to any address, even if they are not an Observer, allowing more Observers to be paid than allocated.
Risk/Impact: As Observers are paid by dividing the total Observer budget by the number of Observers, paying more than the allocated number of Observers will allow the Project creator to drain the contract's funds by paying non-Observers the Observer fee.
Recommendation: Only Observers should be permitted to be paid an Observer fee.
Resolution: The team has not yet addressed this issue.

Finding #17 - ReBakedDAO - High

Description: An Observer can be added twice if they have not been paid, which will result in incorrect Observer fee calculations when paid out.
Risk/Impact: If an Observer is added multiple times, the payout for an Observer will be calculated to be less than it should be. For instance, if only one Observer exists but they have been added twice, their payout will be half of the Observer budget instead of the entire Observer budget.
Recommendation: The same Observer should not be permitted to be added twice.
Resolution: The team has not yet addressed this issue.

Finding #18 - ReBakedDAO - Informational (Resolved)

Description: When calling getMgp() as a Collaborator who was approved after a Dispute, the Package's budget paid is not incremented.
Risk/Impact: As the Package's paid budget amount does not affect functionality, this finding is purely informational.
Recommendation: The Package's budget paid should be incremented in this case.
Resolution: The team has implemented the above recommendation.

Finding #19 - ReBakedDAO - Informational

Description: When removing an Observer before a Package is finished, the budget allocated is incorrectly decremented by the Observer's amountToBePaid. As Observer fees are calculated by the quotient of the Observer budget and the number of Observers, the Package's allocated budget should not be affected if an Observer is removed.
Risk/Impact: As an Observer's amountToBePaid is never set, the Package's allocated budget will be decremented by 0, which will have no impact.
Recommendation: The following line in the removeObserver() function should be removed: 
packageData[projectId_][packageId_[i]].budgetAllocated -= observerData[projectId_][packageId_[i]][observer_].amountToBePaid;

Contract Overview

  • As the contracts are implemented with Solidity 0.8.x, they are protected from overflows.
  • This contract facilitates the creation of Projects, each with their own Packages.
  • Any user can create a new Project at any time, specifying a token and a budget; a Project ID is then generated.
  • If a custom token is used, the Project is immediately approved and started, where the budget amount is transferred from the user to this contract.
  • As amounts are multiplied by 1E18, only tokens with 18 decimals should be used.
  • If the 0 address is used, the owner must manually approve the Project before it can be started.
  • When a Project is started in this case, the TokenFactory contract's deployToken() function is called with the specified budget passed and is assigned as the Project's token.
  • As the TokenFactory contract was not included in the scope of this audit, we are unable to provide an assessment with regards to security or functionality.
  • Once a Project has been started, the Project's creator can create Packages within it. The Package's budget, Observer budget, and bonus are set to the creator-specified values, and it is marked as Active.
  • A 5% fee is taken from the total of these 3 values and transferred from the user to the Treasury address.
  • The creator will also specify how much of the Project's budget that the Package will cost, which will be added to the Project's allocated budget.
  • Each Package within a Project contains "Collaborators" and "Observers" which can be added by the Project creator.
  • When adding a Collaborator, the creator will specify an MGP (Minimum Guaranteed Payment) which is immediately claimable by the Collaborator.
  • The creator can also update a Collaborator's MGP if they have not yet claimed their MGP.
  • The owner or Project creator must provide MGP approval to a Collaborator before they are permitted to withdraw their MGP.
  • Only the owner can deny approval to a Collaborator, which will remove them.
  • Once a Package has been finished, Observers can collect an "Observer fee", which is calculated based on the Package's Observer budget divided by the Package's number of Observers.
  • The creator can also manually pay an Observer their Observer fee or a Collaborator their MGP.
  • The owner can update a Collaborator's "Bonus Score" at any time.
  • A Collaborator can withdraw their Bonus Score amount once the Package is finished.
  • A Package cannot have a total Bonus Score greater than 1 million.
  • The Project creator or an approved Collaborator can "raise a dispute" against a Collaborator for that Package at any time.
  • A Collaborator cannot receive their MGP or Bonus for a Package if a dispute is raised against the Collaborator for that Package.
  • Once a dispute is raised, the owner can either approve or reject the Collaborator.
  • If approved, the Collaborator can once again claim their MGP and any Bonus they have been granted.
  • If rejected, the payment allocated for the Collaborator is instead sent back to the creator of the Project and the Collaborator's Bonus and MGP is deleted.
  • The Project's creator can cancel an unfinished Package at any time.
  • Users can still receive their MGP if a Package is canceled.
  • A Project creator can finish any of its Packages at any time.
  • When a Package is finished, any unused funds are subtracted from the Project's allocated budget.
  • Once all of a Project's Packages have been marked as finished, the Project can be finished.
  • When finishing a Project with a custom token, any unallocated funds are transferred back to the initiator.
  • If the token is not custom, this amount is instead burned from the contract.
  • The owner can update the Treasury address at any time.

Audit Results

Vulnerability Category Notes Result
Arbitrary Jump/Storage Write N/A PASS
Centralization of Control
  • The owner and Project creators have the permissions mentioned above.
  • The owner can grant a bonus to any user at any time, allowing them to withdraw any tokens from the contract.
  • The owner must approve added Collaborators before they can withdraw their MGP.
  • Only the owner can approve or reject raised Disputes.
    		•  FAIL
    Compiler Issues N/A PASS
    Delegate Call to Untrusted Contract N/A PASS
    Dependence on Predictable Variables N/A PASS
    Ether/Token Theft N/A PASS
    Flash Loans N/A PASS
    Front Running N/A PASS
    Improper Events N/A PASS
    Improper Authorization Scheme N/A PASS
    Integer Over/Underflow N/A PASS
    Logical Issues The contract contains multiple logical inconsistencies as described in the findings above. FAIL
    Oracle Issues N/A PASS
    Outdated Compiler Version N/A PASS
    Race Conditions N/A PASS
    Reentrancy N/A PASS
    Signature Issues N/A PASS
    Unbounded Loops N/A PASS
    Unused Code N/A PASS
    Overall Contract Safety   FAIL

    Inheritance Chart

    Smart Contract Audit - Inheritance

    Function Graph

    Smart Contract Audit - Graph

    Functions Overview

    
 ($) = payable function
 # = non-constant function
 
 Int = Internal
 Ext = External
 Pub = Public

_approveCollaborator #
    - [Pub] updatetreasury #
       - modifiers: onlyOwner
    - [Pub] changeFees #
       - modifiers: onlyOwner
    - [Ext] approveProject #
       - modifiers: onlyOwner
    - [Ext] approveProjects #
       - modifiers: noneEmptyBytesArray,onlyOwner
    - [Ext] setBonusScores #
       - modifiers: noneEmptyUintArray,onlyOwner
    - [Ext] approveCollaborator #
       - modifiers: onlyOwner
    - [Ext] raiseDispute #
    - [Ext] approvePayment #
       - modifiers: onlyOwner
    - [Ext] getRejectedPayment
       - modifiers: onlyOwner
    - [Ext] rejectPayment #
       - modifiers: onlyOwner
    - [Ext] approveCollaborators #
       - modifiers: onlyOwner
    - [Ext] finishProject #
       - modifiers: onlyInitiator
    - [Ext] createProject #
       - modifiers: nonZero
    - [Ext] startProject #
       - modifiers: onlyInitiator
    - [Ext] createPackage #
       - modifiers: onlyInitiator,nonZero
    - [Ext] createPackages #
       - modifiers: noneEmptyUintArray,onlyInitiator
    - [Ext] canclePackage #
       - modifiers: onlyInitiator
    - [Pub] payMgp #
       - modifiers: onlyInitiator
    - [Ext] addObserver #
       - modifiers: onlyInitiator
    - [Ext] addObservers #
       - modifiers: onlyInitiator
    - [Ext] addCollaborator #
       - modifiers: onlyInitiator,nonZero
    - [Ext] removeCollaborator #
       - modifiers: onlyInitiator
    - [Ext] removeObserver #
       - modifiers: onlyInitiator
    - [Pub] payObserverFee #
       - modifiers: onlyInitiator
    - [Ext] addCollaborators #
       - modifiers: noneEmptyUintArray,onlyInitiator
    - [Ext] finishPackage #
       - modifiers: onlyInitiator
    - [Pub] getMgp #
       - modifiers: nonReentrant
    - [Ext] getMgps #
       - modifiers: nonReentrant,noneEmptyBytesArray
    - [Ext] getBonus #
       - modifiers: nonReentrant
    - [Ext] getBonuses #
       - modifiers: nonReentrant,noneEmptyBytesArray
    - [Ext] getObserverFee #
       - modifiers: nonReentrant
    - [Ext] getObserverFees #
       - modifiers: nonReentrant,noneEmptyBytesArray

    About SourceHat

    SourceHat has quickly grown to have one of the most experienced and well-equipped smart contract auditing teams in the industry. Our team has conducted 1800+ solidity smart contract audits covering all major project types and protocols, securing a total of over $50 billion U.S. dollars in on-chain value!
    Our firm is well-reputed in the community and is trusted as a top smart contract auditing company for the review of solidity code, no matter how complex. Our team of experienced solidity smart contract auditors performs audits for tokens, NFTs, crowdsales, marketplaces, gambling games, financial protocols, and more!

    Contact us today to get a free quote for a smart contract audit of your project!

    What is a SourceHat Audit?

    Typically, a smart contract audit is a comprehensive review process designed to discover logical errors, security vulnerabilities, and optimization opportunities within code. A SourceHat Audit takes this a step further by verifying economic logic to ensure the stability of smart contracts and highlighting privileged functionality to create a report that is easy to understand for developers and community members alike.

    How Do I Interpret the Findings?

    Each of our Findings will be labeled with a Severity level. We always recommend the team resolve High, Medium, and Low severity findings prior to deploying the code to the mainnet. Here is a breakdown on what each Severity level means for the project:

    • High severity indicates that the issue puts a large number of users' funds at risk and has a high probability of exploitation, or the smart contract contains serious logical issues which can prevent the code from operating as intended.
    • Medium severity issues are those which place at least some users' funds at risk and has a medium to high probability of exploitation.
    • Low severity issues have a relatively minor risk association; these issues have a low probability of occurring or may have a minimal impact.
    • Informational issues pose no immediate risk, but inform the project team of opportunities for gas optimizations and following smart contract security best practices.