QA Report

[code423n4]

Low Risk Issues

Summary Of Findings:

	Issue
1	Zero-check is not performed for address
2	Anyone can withdraw funds for Builder on his behalf (without permission)
3	Value of APR isnt checked to be within range

Details on Findings:

1. Zero-check is not performed for address

In the HomeFi.sol, general address changes are performed only after a zero check. Which is done with the nonZero modifier. But in the setTrustedForwarder function, the new address, _newForwarder is not checked for zero value.

Mitigation would be to add the nonZero modifier in the function as shown below:

    function setTrustedForwarder(address _newForwarder)
        external
        override
        onlyAdmin  
        nonZero(_newForwarder)   //mitigation applied
        noChange(trustedForwarder, _newForwarder)
    {
        trustedForwarder = _newForwarder;
    }

2. Anyone can withdraw funds for Builder on his behalf (without permission)

In Project.sol, the recoverTokens function, sends remaining funds to the builder. But this can be initiated by anyone on his behalf without prior permission. This can be confusing from the Builder's perspective.

Mitigation would be to check if the msg.sender is the builder himself.

3. Value of APR isnt checked to be within range

In Community contract, the value of APR isnt checked to be within a certain range in the publishProject function. This can lead to a large interest to be calculated on the debt amount.

Mitigation would be to check if APR is within a particular range before publishing the project. At least a maximum value should be specified.

Non-Critical Issues

Summary Of Findings:

	Issue
1	Variable name should indicate what it represents
2	Use a newer version of Solidity

Details on Findings:

1. Variable name should indicate what it represents

In Project.sol, in the raiseDispute function, the input data is decoded as shown below:

       (address _project, uint256 _task, , , ) = abi.decode(  // @audit QA could be renamed to taskID
            _data,
            (address, uint256, uint8, bytes, bytes)
        );

The variable _task here represents the taskID. And it should be named as such for better readability. In other parts of the code its done well.

2. Use a newer version of Solidity

The codebase uses Solidity version 0.8.6 which was released in June 2021. Though its not possible to keep up with the version changes of Solidity, its advisable to use a relatively newer version. The current Solidity version is 0.8.15 which was released in June 2022 (one year later than the current version used by the codebase).

Newer versions like 0.8.10 will skip contract existence checks for external calls, if the external call has a return value. This will reduce gas costs.

[jack-the-pug]

Re L-01: Zero-check is not performed for address

What if we need to remove the trustedForwarder?