Rigor Protocol contest - TomJ's results

Community lending and instant payments for new home construction.

General Information

Platform: Code4rena

Start Date: 01/08/2022

Pot Size: $50,000 USDC

Total HM: 26

Participants: 133

Period: 5 days

Judge: Jack the Pug

Total Solo HM: 6

Id: 151

League: ETH

Rigor Protocol

Findings Distribution

Researcher Performance

Rank: 66/133

Findings: 2

Award: $62.65

🌟 Selected for report: 0

πŸš€ Solo Findings: 0

Table of Contents

Low Risk Issues

  • Admin Address Change should be a 2-Step Process
  • Avoid Using ecrecover

Non-critical Issues

  • Performing a Multiplication on the Result of a Division
  • Define Magic Numbers to Constant

 

Low Risk Issues

Admin Address Change should be a 2-Step Process

Issue

High privilege account such as admin / owner is changed with only single process. This can be a concern since an admin / owner role has a high privilege in the contract and mistakenly setting a new admin to an incorrect address will end up losing that privilege.

PoC
HomeFi.sol 157: function replaceAdmin(address _newAdmin) 158: external 159: override 160: onlyAdmin 161: nonZero(_newAdmin) 162: noChange(admin, _newAdmin) 163: { 164: // Replace admin 165: admin = _newAdmin; 166: 167: emit AdminReplaced(_newAdmin); 168: }

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/HomeFi.sol#L157-L168

Mitigation

This can be fixed by implementing 2-step process. We can do this by following. First make the setAdmin function approve a new address as a pending admin. Next that pending admin has to claim the ownership in a separate transaction to be a new admin.

 

Avoid Using ecrecover

Issue

It is best practice to avoide using ecrecover since the ecrecover EVM opcode allows malleable signatures and thus is vulnerable to reply attacks. Since the contract uses nonce, replay attack seems not possible here but it is still recommended to use recover instead of ecrecover. Also ecrecover returns an empty address when the signature is invalid. So it is also best practice to add 0-address check.

Reference: https://docs.soliditylang.org/en/v0.8.15/units-and-global-variables.html?highlight=ecrecover#mathematical-and-cryptographic-functions

PoC
SignatureDecoder.sol: 39: return ecrecover(toEthSignedMessageHash(messageHash), v, r, s);

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/libraries/SignatureDecoder.sol#L39

Mitigation

Use the recover function from OpenZeppelinβ€šΓ„Γ΄s ECDSA library. Or add zero address check.

 

Non-critical Issues

Performing a Multiplication on the Result of a Division

Issue

Solidity integer division might truncate. As a result, performing multiplication before division can sometimes avoid loss of precision. Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#divide-before-multiply

PoC
  1. _noOfDays variable of returnToLender() of Community.sol
685: uint256 _noOfDays = (block.timestamp - 686: _communityProject.lastTimestamp) / 86400; // 24*60*60 687: 688: /// Interest formula = (principal * APR * days) / (365 * 1000) 689: // prettier-ignore 690: uint256 _unclaimedInterest = 691: _lentAmount * 692: _communities[_communityID].projectDetails[_project].apr * 693: _noOfDays / 694: 365000;

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Community.sol#L685-L694

Mitigation

Consider ordering multiplication before division.

 

Define Magic Numbers to Constant

Issue

It is best practice to define magic numbers to constant rather than just using as a magic number. This improves code readability and maintainability.

PoC
  1. Magic Number: 1000
./Community.sol:394: (_projectInstance.lenderFee() + 1000); ./Project.sol:907: ((_amount / 1000) * 1000) == _amount,
  1. Magic Number: 365000
./Community.sol:694: 365000;
  1. Magic Number: 86400
./Community.sol:686: _communityProject.lastTimestamp) / 86400; // 24*60*60
  1. Magic Number: 25000
./Project.sol:60: uint256 public constant override VERSION = 25000;
Mitigation

Define magic numbers to constant.

 

Table of Contents

  • Should Use Unchecked Block where Over/Underflow is not Possible
  • Minimize the Number of SLOADs by Caching State Variable
  • Defined Variables Used Only Once
  • Storage Variables can be Packed into Fewer Storage Slots
  • Use Already Defined Variable
  • Duplicate require() Checks Should be a Modifier or a Function
  • Both named returns and return statement are used
  • Use require instead of &&
  • Internal Function Called Only Once can be Inlined
  • Use Calldata instead of Memory for Read Only Function Parameters
  • Using Elements Smaller than 32 bytes (256 bits) Might Use More Gas
  • Unnecessary Default Value Initialization
  • Store Array's Length as a Variable
  • ++i Costs Less Gas than i++
  • != 0 costs less gass than > 0
  • Use Custom Errors to Save Gas

 

Should Use Unchecked Block where Over/Underflow is not Possible

Issue

Since Solidity 0.8.0, all arithmetic operations revert on overflow and underflow by default. However in places where overflow and underflow is not possible, it is better to use unchecked block to reduce the gas usage. Reference: https://docs.soliditylang.org/en/v0.8.15/control-structures.html#checked-or-unchecked-arithmetic

PoC

Total of 4 issues found.

  1. _reduceDebt() of Community.sol
Wrap line 794 with unchecked since underflow is not possible due to line 792 check 792: require(_lentAndInterest >= _repayAmount, "Community::!Liquid"); 794: _lentAmount = _lentAndInterest - _repayAmount;

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Community.sol#L792-L794

  1. changeOrder() of Project.sol
Wrap line 427 with unchecked since underflow is not possible due to line 425 check 425: if (_newCost < _taskCost) { 427: uint256 _withdrawDifference = _taskCost - _newCost;

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L425-L427

  1. allocateFunds() of Project.sol
Wrap line 616 with unchecked since underflow is not possible due to line 614 check 614: if (_costToAllocate >= _taskCost) { 616: _costToAllocate -= _taskCost;

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L614-L616

  1. allocateFunds() of Project.sol
Wrap line 663 with unchecked since underflow is not possible due to line 661 check 661: if (_costToAllocate >= _taskCost) { 663: _costToAllocate -= _taskCost;

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L661-L663

Mitigation

Wrap the code with uncheck like described in above PoC.

 

Minimize the Number of SLOADs by Caching State Variable

Issue

SLOADs cost 100 gas where MLOADs/MSTOREs cost only 3 gas. Whenever function reads storage value more than once, it should be cached to save gas.

PoC
  1. Cache homeFi storage variable of createProject() of ProjectFactory.sol 2 SLOADs to 1 SLOAD, 1 MSTORE and 2 MLOAD

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/ProjectFactory.sol#L84 https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/ProjectFactory.sol#L90

  1. Cache projectCount storage variable of createProject() of HomeFi.sol 3 SLOADs to 1 SLOAD, 1 MSTORE and 3 MLOAD

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/HomeFi.sol#L228-L231

  1. Cache projectCount storage variable of mintNFT() of HomeFi.sol 4 SLOADs to 1 SLOAD, 1 MSTORE and 4 MLOAD

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/HomeFi.sol#L292-L296

  1. Cache tasks[_taskID] storage variable of setComplete() of Project.sol 3 SLOADs to 1 SLOAD, 1 MSTORE and 3 MLOAD

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L350 https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L354-L355

  1. Cache tasks[_taskID] storage variable of changeOrder() of Project.sol 8 SLOADs to 1 SLOAD, 1 MSTORE and 8 MLOAD

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L409 https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L423 https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L449 https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L452 https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L464 https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L470 https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L474 https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L485

  1. Cache tasks[_task] storage variable of raiseDispute() of Project.sol 2 SLOADs to 1 SLOAD, 1 MSTORE and 2 MLOAD

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L524 https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L528

  1. Cache tasks[id] storage variable of getTask() of Project.sol 3 SLOADs to 1 SLOAD, 1 MSTORE and 3 MLOAD

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L553-L555

  1. Cache builder storage variable of lendToProject() of Project.sol 2 SLOADs to 1 SLOAD, 1 MSTORE and 2 MLOAD

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L190 https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L204

  1. Cache taskCount storage variable of allocateFunds() of Project.sol 5 or more SLOADs to 1 SLOAD, 1 MSTORE and 5 or more MLOAD

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L592 https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L648 https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L650 https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L681 https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L682

Mitigation

When certain state variable is read more than once, cache it to local variable to save gas.

 

Defined Variables Used Only Once

Issue

Certain variables is defined even though they are used only once. Remove these unnecessary variables to save gas. For cases where it will reduce the readability, one can use comments to help describe what the code is doing.

PoC
  1. Remove '_totalAllocated' variable of changeOrder() Project.sol https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L420 https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L438 Delete line 420 and replace line 438 to line shown below
else if (totalLent - totalAllocated >= _newCost - _taskCost) {
Mitigation

Don't define variable that is used only once.

 

Storage Variables can be Packed into Fewer Storage Slots

Issue

The order of storage variables can be reordered in a way to reduce the usage amount of storage slots.

Reference from solidity documentation: Finally, in order to allow the EVM to optimize for this, ensure that you try to order your storage variables and struct members such that they can be packed tightly. For example, declaring your storage variables in the order of uint128, uint128, uint256 instead of uint128, uint256, uint128, as the former will only take up two slots of storage whereas the latter will take up three.

https://docs.soliditylang.org/en/v0.8.15/internals/layout_in_storage.html#layout-of-state-variables-in-storage

PoC

Total of 1 issue found.

  1. Project.sol We can save 1 storage slot by reordering it like below. Move bool variable (1 byte size) to pack it within 1 slot (32 bytes size) with address variable (20 bytes size) and other bool variable.
Originally
66:    address public override contractor; // 20 bytes
68:    bool public override contractorConfirmed; // 1 byte
70:    uint256 public override hashChangeNonce; // 32 bytes
72:    uint256 public override totalLent; // 32 bytes
74:    uint256 public override totalAllocated; // 32 bytes
76:    uint256 public override taskCount; // 32 bytes
78:    bool public override contractorDelegated; // 1 byte
80:    uint256 public override lastAllocatedTask; // 32 bytes
82:    uint256 public override lastAllocatedChangeOrderTask; // 32 bytes
84:    mapping(address => mapping(bytes32 => bool)) public override approvedHashes; // 32 bytes
Change to
    address public override contractor; // 20 bytes
    bool public override contractorConfirmed; // 1 byte
    bool public override contractorDelegated; // 1 byte
    uint256 public override hashChangeNonce; // 32 bytes
    uint256 public override totalLent; // 32 bytes
    uint256 public override totalAllocated; // 32 bytes
    uint256 public override taskCount; // 32 bytes
    uint256 public override lastAllocatedTask; // 32 bytes
    uint256 public override lastAllocatedChangeOrderTask; // 32 bytes
    mapping(address => mapping(bytes32 => bool)) public override approvedHashes; // 32 bytes

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L65-L84

Mitigation

Reorder storage variables like shown in above PoC.

 

Use Already Defined Variable

Issue

Use already defined variable rather than reading the storage variable again and wasting gas.

PoC
  1. lendToProject() of Community.sol Use cached variable of _sender instead of reading the storage variable https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Community.sol#L380

Change _msgSender() to _sender

Originally
443: _currency.safeTransferFrom(_msgSender(), homeFi.treasury(), _lenderFee);
446: _currency.safeTransferFrom(_msgSender(), _project, _amountToProject);

Change To
443: _currency.safeTransferFrom(_sender, homeFi.treasury(), _lenderFee);
446: _currency.safeTransferFrom(_sender, _project, _amountToProject);
Mitigation

Use the already defined variable like shown in above PoC.

 

Duplicate require() Checks Should be a Modifier or a Function

Issue

Since below require checks are used more than once, I recommend making these to a modifier or a function.

PoC
./Project.sol:132: require(_projectAddress == address(this), "Project::!projectAddress"); ./Project.sol:241: require(_projectAddress == address(this), "Project::!projectAddress");
./Project.sol:176: require(_nonce == hashChangeNonce, "Project::!Nonce"); ./Project.sol:277: require(_nonce == hashChangeNonce, "Project::!Nonce");
./Project.sol:406: require(_project == address(this), "Project::!projectAddress"); ./Project.sol:511: require(_project == address(this), "Project::!projectAddress");
Mitigation

I recommend making duplicate require statement into modifier or a function.

 

Both named returns and return statement are used

Issue

In some function return statement are used even though named returns is set. This is redundant because return statement is not needed when using named returns and named returns is not needed when using return statement. Removing unused named returns variable in below code can save gas and improve code readability.

PoC

1, Remove returns variable "sender" of _msgSender() Community.sol https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Community.sol#L903

Mitigation

Remove unused named returns variable as mentioned in above PoC.

 

Use require instead of &&

Issue

When there are multiple conditions in require statement, break down the require statement into multiple require statements instead of using && can save gas.

PoC

Total of 4 issues found.

./Community.sol:353-357: require(_lendingNeeded >= _communityProject.totalLent && _lendingNeeded <= IProject(_project).projectCost(), "Community::invalid lending"); ./Disputes.sol:61-65: require(_disputeID < disputeCount && disputes[_disputeID].status == Status.Active, "Disputes::!Resolvable"); ./Disputes.sol:106-109: require(_actionType > 0 && _actionType <= uint8(ActionType.TaskPay), "Disputes::!ActionType"); ./Project.sol:189-192: require(_sender == builder || _sender == homeFi.communityContract(), "Project::!Builder&&!Community");
Mitigation

Break down into several require statement. For example,

require(_lendingNeeded >= _communityProject.totalLent); require(_lendingNeeded <= IProject(_project).projectCost(), "Community::invalid lending");

 

Internal Function Called Only Once Can be Inlined

Issue

Certain function is defined even though it is called only once. Inline it instead to where it is called to avoid usage of extra gas.

PoC

Total of 3 issues found.

  1. resolveHandler() of Disputes.sol
./Disputes.sol:207:    function resolveHandler(uint256 _disputeID) internal {

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Disputes.sol#L207

resolveHandler function called only once at line 149 https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Disputes.sol#L149

  1. autoWithdraw() of Project.sol
./Project.sol:770:    function autoWithdraw(uint256 _amount) internal {

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L770

autoWithdraw function called only once at line 435 https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L435

  1. _replaceImplementation() of HomeFiProxy.sol
197:    function _replaceImplementation(
198:        bytes2 _contractName,
199:        address _contractAddress
200:    ) internal nonZero(_contractAddress) {

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/HomeFiProxy.sol#L197-L200

_replaceImplementation function called only once at line 137 https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/HomeFiProxy.sol#L137

Mitigation

I recommend to not define above functions and instead inline it at place it is called.

 

Use Calldata instead of Memory for Read Only Function Parameters

Issue

It is cheaper gas to use calldata than memory if the function parameter is read only. Calldata is a non-modifiable, non-persistent area where function arguments are stored, and behaves mostly like memory. More details on following link.

link: https://docs.soliditylang.org/en/v0.8.15/types.html#data-location

PoC

Total of 2 issues found.

./Community.sol:488: bytes memory _details ./HomeFi.sol:210: function createProject(bytes memory _hash, address _currency)
Mitigation

Change memory to calldata

 

Using Elements Smaller than 32 bytes (256 bits) Might Use More Gas

Issue

Since EVM operates on 32 bytes at a time, if the element is smaller than that, the EVM must use more operations in order to reduce the elements from 32 bytes to specified size.

Reference: https://docs.soliditylang.org/en/v0.8.15/internals/layout_in_storage.html

PoC

Total of 8 issues found.

./DebtToken.sol:16: uint8 internal _decimals; ./DebtToken.sol:47: uint8 decimals_ ./DebtToken.sol:82: function decimals() public view virtual override returns (uint8) { ./SignatureDecoder.sol:29: uint8 v; ./SignatureDecoder.sol:65: uint8 v, ./Disputes.sol:100: uint8 _actionType,
Mitigation

I suggest using uint256 instead of anything smaller or downcast where needed.

 

Unnecessary Default Value Initialization

Issue

When variable is not initialized, it will have its default values. For example, 0 for uint, false for bool and address(0) for address. Reference: https://docs.soliditylang.org/en/v0.8.15/control-structures.html#scoping-and-declarations

PoC

Total of 8 issues found.

./Community.sol:624: for (uint256 i = 0; i < _communities[_communityID].memberCount; i++) { ./Tasks.sol:181: for (uint256 i = 0; i < _length; i++) _alerts[i] = _self.alerts[i]; ./Project.sol:248: for (uint256 i = 0; i < _length; i++) { ./Project.sol:311: for (uint256 i = 0; i < _length; i++) { ./Project.sol:322: for (uint256 i = 0; i < _length; i++) { ./HomeFiProxy.sol:87: for (uint256 i = 0; i < _length; i++) { ./HomeFiProxy.sol:136: for (uint256 i = 0; i < _length; i++) { ./Project.sol:412: bool _unapproved = false;
Mitigation

I suggest removing default value initialization. For example,

  • bool _unapproved;

 

Store Array's Length as a Variable

Issue

By storing an array's length as a variable before the for-loop, can save 3 gas per iteration.

PoC

Total of 1 issue found.

./Project.sol:603: for (; i < _changeOrderedTask.length; i++) {
Mitigation

Store array's length as a variable before looping it. For example, I suggest changing it to

uint256 length = _changeOrderedTask.length; for (; i < length; i++) {

 

++i Costs Less Gas than i++

Issue

Prefix increments/decrements (++i or --i) costs cheaper gas than postfix increment/decrements (i++ or i--).

PoC

Total of 11 issues found.

./Community.sol:624: for (uint256 i = 0; i < _communities[_communityID].memberCount; i++) { ./Tasks.sol:181: for (uint256 i = 0; i < _length; i++) _alerts[i] = _self.alerts[i]; ./Project.sol:248: for (uint256 i = 0; i < _length; i++) { ./Project.sol:311: for (uint256 i = 0; i < _length; i++) { ./Project.sol:322: for (uint256 i = 0; i < _length; i++) { ./Project.sol:368: for (uint256 _taskID = 1; _taskID <= _length; _taskID++) { ./Project.sol:603: for (; i < _changeOrderedTask.length; i++) { ./Project.sol:650: for (++j; j <= taskCount; j++) { ./Project.sol:710: for (uint256 _taskID = 1; _taskID <= _length; _taskID++) { ./HomeFiProxy.sol:87: for (uint256 i = 0; i < _length; i++) { ./HomeFiProxy.sol:136: for (uint256 i = 0; i < _length; i++) {
Mitigation

Change it to postfix increments/decrements. It saves 6 gas per loop. For example,

for (uint256 i = 0; i < _communities[_communityID].memberCount; ++i) {

 

!= 0 costs less gass than > 0

Issue

!= 0 costs less gas when optimizer is enabled and is used for unsigned integers in require statement.

PoC

Total of 2 issues found.

./Community.sol:764: require(_repayAmount > 0, "Community::!repay"); ./Project.sol:195: require(_cost > 0, "Project::!value>0");
Mitigation

I suggest changing > 0 to != 0 For example:

require(_repayAmount != 0, "Community::!repay");

 

Use Custom Errors to Save Gas

Issue

Custom errors from Solidity 0.8.4 are cheaper than revert strings. Details are explained here: https://blog.soliditylang.org/2021/04/21/custom-errors/

PoC

Total of 74 issues found.

./ProjectFactory.sol:36: require(_address != address(0), "PF::0 address"); ./ProjectFactory.sol:64-67: require(_msgSender() == IHomeFi(homeFi).admin(), "ProjectFactory::!Owner"); ./ProjectFactory.sol:84: require(_msgSender() == homeFi, "PF::!HomeFiContract"); ./DebtToken.sol:31-34: require(communityContract == _msgSender(), "DebtToken::!CommunityContract"); ./DebtToken.sol:50: require(_communityContract != address(0), "DebtToken::0 address"); ./Community.sol:69: require(_address != address(0), "Community::0 address"); ./Community.sol:75: require(_msgSender() == homeFi.admin(), "Community::!admin"); ./Community.sol:81-84: require(projectPublished[_project] == _communityID, "Community::!published"); ./Community.sol:90-93: require(_msgSender() == IProject(_project).builder(), "Community::!Builder"); ./Community.sol:131-134: require(!restrictedToAdmin || _sender == homeFi.admin(), "Community::!admin"); ./Community.sol:159-162: require(_communities[_communityID].owner == _msgSender(), "Community::!owner"); ./Community.sol:191-192: require(!_community.isMember[_newMemberAddr], "Community::Member Exists"); ./Community.sol:235-238: require(_publishNonce == _community.publishNonce, "Community::invalid publishNonce"); ./Community.sol:241: require(homeFi.isProjectExist(_project), "Community::Project !Exists"); ./Community.sol:248: require(_community.isMember[_builder], "Community::!Member"); ./Community.sol:251-254: require(_projectInstance.currency() == _community.currency, "Community::!Currency"); ./Community.sol:312-315: require(!_communityProject.publishFeePaid, "Community::publish fee paid"); ./Community.sol:347-350: require(_communityProject.publishFeePaid, "Community::publish fee !paid"); ./Community.sol:353-357: require(_lendingNeeded >= _communityProject.totalLent && _lendingNeeded <= IProject(_project).projectCost(), "Community::invalid lending"); ./Community.sol:384-387: require(_sender == _communities[_communityID].owner, "Community::!owner"); ./Community.sol:400-409: require(_amountToProject <= _communities[_communityID].projectDetails[_project].lendingNeeded - _communities[_communityID].projectDetails[_project].totalLent, "Community::lending>needed"); ./Community.sol:491-494: require(_msgSender() == _communities[_communityID].owner, "Community::!Owner"); ./Community.sol:536: require(_builder == _projectInstance.builder(), "Community::!Builder"); ./Community.sol:539-542: require(_lender == _communities[_communityID].owner, "Community::!Owner"); ./Community.sol:557: require(!restrictedToAdmin, "Community::restricted"); ./Community.sol:568: require(restrictedToAdmin, "Community::!restricted"); ./Community.sol:764: require(_repayAmount > 0, "Community::!repay"); ./Community.sol:792: require(_lentAndInterest >= _repayAmount, "Community::!Liquid"); ./Community.sol:886-889: require(_recoveredSignature == _address || approvedHashes[_address][_hash], "Community::invalid signature"); ./Tasks.sol:44: require(_self.state == TaskStatus.Inactive, "Task::active"); ./Tasks.sol:50: require(_self.state == TaskStatus.Active, "Task::!Active"); ./Tasks.sol:56-59: require(_self.alerts[uint256(Lifecycle.TaskAllocated)], "Task::!funded"); ./Tasks.sol:124: require(_self.subcontractor == _sc, "Task::!SC"); ./Disputes.sol:39: require(_address != address(0), "Disputes::0 address"); ./Disputes.sol:46: require(homeFi.admin() == _msgSender(), "Disputes::!Admin"); ./Disputes.sol:52: require(homeFi.isProjectExist(_msgSender()), "Disputes::!Project"); ./Disputes.sol:61-65: require(_disputeID < disputeCount && disputes[_disputeID].status == Status.Active, "Disputes::!Resolvable"); ./Disputes.sol:106-109: require(_actionType > 0 && _actionType <= uint8(ActionType.TaskPay), "Disputes::!ActionType"); ./Disputes.sol:183: require(_result, "Disputes::!Member"); ./HomeFi.sol:73: require(admin == _msgSender(), "HomeFi::!Admin"); ./HomeFi.sol:78: require(_address != address(0), "HomeFi::0 address"); ./HomeFi.sol:84: require(_oldAddress != _newAddress, "HomeFi::!Change"); ./HomeFi.sol:142: require(!addrSet, "HomeFi::Set"); ./HomeFi.sol:191: require(lenderFee != _newLenderFee, "HomeFi::!Change"); ./HomeFi.sol:255-260: require(_currency == tokenCurrency1 || _currency == tokenCurrency2 || _currency == tokenCurrency3, "HomeFi::!Currency"); ./Project.sol:123: require(!contractorConfirmed, "Project::GC accepted"); ./Project.sol:132: require(_projectAddress == address(this), "Project::!projectAddress"); ./Project.sol:135: require(_contractor != address(0), "Project::0 address"); ./Project.sol:150: require(_msgSender() == builder, "Project::!B"); ./Project.sol:153: require(contractor != address(0), "Project::0 address"); ./Project.sol:176: require(_nonce == hashChangeNonce, "Project::!Nonce"); ./Project.sol:189-192: require(_sender == builder || _sender == homeFi.communityContract(), "Project::!Builder&&!Community"); ./Project.sol:195: require(_cost > 0, "Project::!value>0"); ./Project.sol:199-202: require(projectCost() >= uint256(_newTotalLent), "Project::value>required"); ./Project.sol:238: require(_taskCount == taskCount, "Project::!taskCount"); ./Project.sol:241: require(_projectAddress == address(this), "Project::!projectAddress"); ./Project.sol:245: require(_length == _taskCosts.length, "Project::Lengths !match"); ./Project.sol:277: require(_nonce == hashChangeNonce, "Project::!Nonce"); ./Project.sol:301-304: require(_msgSender() == builder || _msgSender() == contractor, "Project::!Builder||!GC"); ./Project.sol:308: require(_length == _scList.length, "Project::Lengths !match"); ./Project.sol:341: require(_projectAddress == address(this), "Project::!Project"); ./Project.sol:369: require(tasks[_taskID].getState() == 3, "Project::!Complete"); ./Project.sol:406: require(_project == address(this), "Project::!projectAddress"); ./Project.sol:511: require(_project == address(this), "Project::!projectAddress"); ./Project.sol:515-518: require(signer == builder || signer == contractor, "Project::!(GC||Builder)"); ./Project.sol:521-526: require(signer == builder || signer == contractor || signer == tasks[_task].subcontractor, "Project::!(GC||Builder||SC)"); ./Project.sol:530: require(getAlerts(_task)[2], "Project::!SCConfirmed"); ./Project.sol:753: require(_sc != address(0), "Project::0 address"); ./Project.sol:886-889: require(_recoveredSignature == _address || approvedHashes[_address][_hash], "Project::invalid signature"); ./Project.sol:906-909: require(((_amount / 1000) * 1000) == _amount, "Project::Precision>=1000"); ./HomeFiProxy.sol:41: require(_address != address(0), "Proxy::0 address"); ./HomeFiProxy.sol:81: require(_length == _implementations.length, "Proxy::Lengths !match"); ./HomeFiProxy.sol:105-108: require(contractAddress[_contractName] == address(0), "Proxy::Name !OK"); ./HomeFiProxy.sol:133: require(_length == _contractAddresses.length, "Proxy::Lengths !match");
Mitigation

I suggest implementing custom errors to save gas.

 

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax Β© 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter