QA Report

[code423n4]

[L-01] The function changeOrder doesn't follow the check-effects-interaction pattern

Impact

Wherever possible, the contract should follow the check-effects-interaction pattern.

On the changeOrder function, the state is modified tasks[_taskID].cost = _newCost after an external call is made autoWithdraw(_withdrawDifference).

File: contracts/Project.sol
435: autoWithdraw(_withdrawDifference);

464: tasks[_taskID].cost = _newCost;

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

If the external call throws, the state should be reverted. Also, since the value for tasks[_taskID].cost is already cached on the _taskCost variable, updating the state before the external call shouldn't impact the bussiness logic.

Recommended Mitigation Steps

Even if the function is already using the nonReetrant modifier, I would still recommend updating the state before making external calls.

$ git diff --no-index Project.sol.orig Project.sol
diff --git a/Project.sol.orig b/Project.sol
index 7e0ae45..2644d87 100644
--- a/Project.sol.orig
+++ b/Project.sol
@@ -411,6 +411,9 @@ contract Project is
         // Local variable indicating if subcontractor is already unapproved.
         bool _unapproved = false;

+        // Store new cost for the task
+        tasks[_taskID].cost = _newCost;
+
         // If task cost is to be changed.
         if (_newCost != _taskCost) {
             // Check new task cost precision. Revert if too precise.
@@ -460,9 +463,6 @@ contract Project is
                 }
             }

-            // Store new cost for the task
-            tasks[_taskID].cost = _newCost;
-
             emit ChangeOrderFee(_taskID, _newCost);
         }

[L-02] Lack of zero address check for critical input parameters

Critical inputs of type address should not be able to unintentionally receive the value zero.

Not checking agaist input address zero can cause misuse of tokens and force redeployments.

File: contracts/Project.sol
function initialize(
    address _currency,
    address _sender,
    address _homeFiAddress
) external override initializer {
    // Initialize variables
    homeFi = IHomeFi(_homeFiAddress);
    disputes = homeFi.disputesContract();
    lenderFee = homeFi.lenderFee();
    builder = _sender;
    currency = IDebtToken(_currency);
}

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

[NC-01] Avoid typo

There's a typo on the following comment.

File: contracts/libraries/SignatureDecoder.sol
8: * @notice Decodes signatures that a encoded as bytes

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

[NC-02] Missing return for natspec

The function signatureSplit should contain a description of the returned rsv data. Consider adding the @return tag to ensure natspec best practices.

File: contracts/libraries/SignatureDecoder.sol
/**
* @dev Divides bytes signature into `uint8 v, bytes32 r, bytes32 s`.
* @dev Make sure to perform a bounds check for @param pos, to avoid out of bounds access on @param signatures

* @param pos which signature to read. A prior bounds check of this parameter should be performed, to avoid out of bounds access
* @param signatures concatenated rsv signatures
*/
function signatureSplit(bytes memory signatures, uint256 pos)
    internal
    pure
    returns (
        uint8 v,
        bytes32 r,
        bytes32 s
    )
{

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

[NC-03] Public functions not consumed by the contract should be declared external

File: contracts/Community.sol
709: function isTrustedForwarder(address _forwarder)

https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/Community.sol

[NC-04] Use time units for calculations involving time

Instead of using the number of seconds, solidity allows the usage of time units such as days and weeks to improve redability.

File: contracts/Community.sol
686: _communityProject.lastTimestamp) / 86400; // 24*60*60

https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/Community.sol