Rigor Protocol contest - hyh's results

Lines of code

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Community.sol#L509-L552

Vulnerability details

checkSignatureValidity() verification by signature do not utilize nonces and can be tricked by using owner / builder signatures from earlier calls. Namely, while checkSignatureValidity's approvedHashes based way can used only once as it deletes the corresponding array entry, the _signature based logic can be reused and itself contains no nonce, just validating that the signature for the constant message is from the desired actor. This validation will go through for all the subsequent calls, which provides a way to bypass the verification by supplying previously recorded signature for the same hash used in a different operation.

Signatory replay is a low level issue that opens up a range of attack surfaces. Currently all the uses besides escrow() utilize nonces, so the impact consists of escrow() calls being repeated as there is no nonce in the data, reducing the debt up to zero.

Proof of Concept

escrow() doesn't has nonce in the _hash, so can be rerun with the same _data:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Community.sol#L509-L552

    function escrow(bytes calldata _data, bytes calldata _signature)
        external
        virtual
        override
        whenNotPaused
    {
        // Decode params from _data
        (
            uint256 _communityID,
            address _builder,
            address _lender,
            address _agent,
            address _project,
            uint256 _repayAmount,
            bytes memory _details
        ) = abi.decode(
                _data,
                (uint256, address, address, address, address, uint256, bytes)
            );

        // Compute hash from bytes
        bytes32 _hash = keccak256(_data);

        // Local instance of variable. For saving gas.
        IProject _projectInstance = IProject(_project);

        // Revert if decoded builder is not decoded project's builder
        require(_builder == _projectInstance.builder(), "Community::!Builder");

        // Revert if decoded _communityID's owner is not decoded _lender
        require(
            _lender == _communities[_communityID].owner,
            "Community::!Owner"
        );

        // check signatures
        checkSignatureValidity(_lender, _hash, _signature, 0); // must be lender
        checkSignatureValidity(_builder, _hash, _signature, 1); // must be builder
        checkSignatureValidity(_agent, _hash, _signature, 2); // must be agent or escrow

        // Internal call to reduce debt
        _reduceDebt(_communityID, _project, _repayAmount, _details);
        emit DebtReducedByEscrow(_agent);
    }

checkSignatureValidity() allows two ways of verification, where approvedHashes based way can used only once, while _signature based approach can be reused repeatedly:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Community.sol#L871-L893

    function checkSignatureValidity(
        address _address,
        bytes32 _hash,
        bytes memory _signature,
        uint256 _signatureIndex
    ) internal virtual {
        // Decode signer
        address _recoveredSignature = SignatureDecoder.recoverKey(
            _hash,
            _signature,
            _signatureIndex
        );

        // Revert if decoded signer does not match expected address
        // Or if hash is not approved by the expected address.
        require(
            _recoveredSignature == _address || approvedHashes[_address][_hash],
            "Community::invalid signature"
        );

        // Delete from approvedHash. So that signature cannot be reused.
        delete approvedHashes[_address][_hash];
    }

This holds as SignatureDecoder's recoverKey() can be run with the same signature, there is no nonce, so it can be run for the same messageHash:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/libraries/SignatureDecoder.sol#L20-L41

    function recoverKey(
        bytes32 messageHash,
        bytes memory messageSignatures,
        uint256 pos
    ) internal pure returns (address) {
        if (messageSignatures.length % 65 != 0) {
            return (address(0));
        }

        uint8 v;
        bytes32 r;
        bytes32 s;
        (v, r, s) = signatureSplit(messageSignatures, pos);

        // If the version is correct return the signer address
        if (v != 27 && v != 28) {
            return (address(0));
        } else {
            // solium-disable-next-line arg-overflow
            return ecrecover(toEthSignedMessageHash(messageHash), v, r, s);
        }
    }

Recommended Mitigation Steps

To target the root source nonce has to be presented in all the signature verifications, invalidating all the subsequent uses.

This way consider ensuring that in all the instances checkSignatureValidity() is called for the hash that includes a specific nonce for the functionality. Whenever this doesn't hold the signature replay is possible with full consequences being reasonably hard to track.

Lines of code

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Project.sol#L295-L316

Vulnerability details

Malicious builder/contractor can change the subcontractor for any task even if all the terms was agreed upon and work was started/finished, but the task wasn't set to completed yet, i.e. it's SCConfirmed, getAlerts(_taskID)[2] == true. This condition is not checked by inviteSC().

For example, a contractor can create a subcontractor of her own and front run valid setComplete() call with a sequence of inviteSC(task, own_subcontractor) -> setComplete() with a signatory from the own_subcontractor, stealing the task budget from the subcontractor who did the job. Contractor will not breach any duties with the community as the task will be done, while raiseDispute() will not work for a real subcontractor as the task record will be already changed.

Setting the severity to be high as this creates an attack vector to fully steal task budget from the subcontractor as at the moment of any valid setComplete() call the task budget belongs to subcontractor as the job completion is already verified by all the parties.

Proof of Concept

inviteSC() requires either builder or contractor to call for the change and verify nothing else:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Project.sol#L295-L316

    /// @inheritdoc IProject
    function inviteSC(uint256[] calldata _taskList, address[] calldata _scList)
        external
        override
    {
        // Revert if sender is neither builder nor contractor.
        require(
            _msgSender() == builder || _msgSender() == contractor,
            "Project::!Builder||!GC"
        );

        // Revert if taskList array length not equal to scList array length.
        uint256 _length = _taskList.length;
        require(_length == _scList.length, "Project::Lengths !match");

        // Invite subcontractor for each task.
        for (uint256 i = 0; i < _length; i++) {
            _inviteSC(_taskList[i], _scList[i], false);
        }

        emit MultipleSCInvited(_taskList, _scList);
    }

_inviteSC() only checks non-zero address and calls inviteSubcontractor():

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Project.sol#L747-L762

    function _inviteSC(
        uint256 _taskID,
        address _sc,
        bool _emitEvent
    ) internal {
        // Revert if sc to invite is address 0
        require(_sc != address(0), "Project::0 address");

        // Internal call to tasks invite contractor
        tasks[_taskID].inviteSubcontractor(_sc);

        // If `_emitEvent` is true (called via changeOrder) then emit event
        if (_emitEvent) {
            emit SingleSCInvited(_taskID, _sc);
        }
    }

inviteSubcontractor() just sets the new value:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/libraries/Tasks.sol#L106-L111

    function inviteSubcontractor(Task storage _self, address _sc)
        internal
        onlyInactive(_self)
    {
        _self.subcontractor = _sc;
    }

Task is paid only on completion by setComplete():

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Project.sol#L349-L356

        // Mark task as complete. Only works when task is active.
        tasks[_taskID].setComplete();

        // Transfer funds to subcontractor.
        currency.safeTransfer(
            tasks[_taskID].subcontractor,
            tasks[_taskID].cost
        );

This way the absence of getAlerts(_taskID)[2] check and checkSignatureTask() call in inviteSC() provides a way for builder or contractor to steal task budget from a subcontractor.

Recommended Mitigation Steps

Consider calling checkSignatureTask() when getAlerts(_taskID)[2] is true, schematically:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Project.sol#L310-L313

        // Invite subcontractor for each task.
        for (uint256 i = 0; i < _length; i++) {
+           if (getAlerts(_taskList[i])[2])
+               checkSignatureTask(_data_with_scList[i], _signature, _taskList[i]);        
            _inviteSC(_taskList[i], _scList[i], false);
        }

This approach is already implemented in changeOrder() where _newSC is a part of hash that has to be signed by all the parties:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Project.sol#L386-L403

    function changeOrder(bytes calldata _data, bytes calldata _signature)
        external
        override
        nonReentrant
    {
        // Decode params from _data
        (
            uint256 _taskID,
            address _newSC,
            uint256 _newCost,
            address _project
        ) = abi.decode(_data, (uint256, address, uint256, address));

        // If the sender is disputes contract, then do not check for signatures.
        if (_msgSender() != disputes) {
            // Check for required signatures.
            checkSignatureTask(_data, _signature, _taskID);
        }

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Project.sol#L477-L481

            // If new subcontractor is not zero address.
            if (_newSC != address(0)) {
                // Invite the new subcontractor for the task.
                _inviteSC(_taskID, _newSC, true);
            }

checkSignatureTask() checks all the signatures:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Project.sol#L855-L861

            // When builder has not delegated rights to contractor
            else {
                // Check for B, SC and GC signatures
                checkSignatureValidity(builder, _hash, _signature, 0);
                checkSignatureValidity(contractor, _hash, _signature, 1);
                checkSignatureValidity(_sc, _hash, _signature, 2);
            }

    function inviteSubcontractor(Task storage _self, address _sc)
        internal
        onlyInactive(_self)
    {

            // If tasks are already allocated with old cost.
            if (tasks[_taskID].alerts[1]) {
                // If new task cost is less than old task cost.
                if (_newCost < _taskCost) {
                    // Find the difference between old - new.
                    uint256 _withdrawDifference = _taskCost - _newCost;

                    // Reduce this difference from total cost allocated.
                    // As the same task is now allocated with lesser cost.
                    totalAllocated -= _withdrawDifference;

                    // Withdraw the difference back to builder's account.
                    // As this additional amount may not be required by the project.
                    autoWithdraw(_withdrawDifference);
                }
                // If new cost is more than task cost but total lent is enough to cover for it.
                else if (totalLent - _totalAllocated >= _newCost - _taskCost) {
                    // Increase the difference of new cost and old cost to total allocated.
                    totalAllocated += _newCost - _taskCost;
                }
                // If new cost is more than task cost and totalLent is not enough.
                else {
                    // Un-confirm SC, mark task as inactive, mark allocated as false, mark lifecycle as None

                    // Mark task as inactive by unapproving subcontractor.
                    // As subcontractor can only be approved if task is allocated
                    _unapproved = true;
                    tasks[_taskID].unApprove();

                    // Mark task as not allocated.
                    tasks[_taskID].unAllocateFunds();

                    // Reduce total allocation by old task cost.
                    // As as needs to go though funding process again.
                    totalAllocated -= _taskCost;

                    // Add this task to _changeOrderedTask array. These tasks will be allocated first.
                    _changeOrderedTask.push(_taskID);
                }
            }

    /**
     * @dev Set a task as un accepted/approved for SC

     * @dev modifier onlyActive

     * @param _self Task the task being set as funded
     */
    function unApprove(Task storage _self) internal {
        // State/ lifecycle //
        _self.alerts[uint256(Lifecycle.SCConfirmed)] = false;
        _self.state = TaskStatus.Inactive;
    }

        // Decode params from _data
        (
            uint256 _taskID,
            address _newSC,
            uint256 _newCost,
            address _project
        ) = abi.decode(_data, (uint256, address, uint256, address));

        // If the sender is disputes contract, then do not check for signatures.
        if (_msgSender() != disputes) {
            // Check for required signatures.
            checkSignatureTask(_data, _signature, _taskID);
        }

            // When builder has not delegated rights to contractor
            else {
                // Check for B, SC and GC signatures
                checkSignatureValidity(builder, _hash, _signature, 0);
                checkSignatureValidity(contractor, _hash, _signature, 1);
                checkSignatureValidity(_sc, _hash, _signature, 2);
            }

    function unConfirm(Task storage _self) internal {
        // State/ lifecycle //
        _self.alerts[uint256(Lifecycle.SCConfirmed)] = false;
    }

    function deActivate(Task storage _self) internal {
        // State/ lifecycle //
        _self.state = TaskStatus.Inactive;
    }

    /// @dev only allow unconfirmed tasks.
    modifier onlyUnconfirmed(Task storage _self) {
        require(
            !_self.alerts[uint256(Lifecycle.SCConfirmed)],
            "Task::SCConfirmed"
        );
        _;
    }

    function inviteSubcontractor(Task storage _self, address _sc)
        internal
-       onlyInactive(_self)
+       onlyUnconfirmed(_self)
    {
        _self.subcontractor = _sc;
    }

    function acceptInvitation(Task storage _self, address _sc)
        internal
-       onlyInactive(_self)
+       onlyUnconfirmed(_self)
    {
        // Prerequisites //
        require(_self.subcontractor == _sc, "Task::!SC");

        // State/ lifecycle //
        _self.alerts[uint256(Lifecycle.SCConfirmed)] = true;
        _self.state = TaskStatus.Active;
    }

    /// @dev only allow inactive tasks. Task is inactive if SC is unconfirmed.
    modifier onlyInactive(Task storage _self) {
        require(_self.state == TaskStatus.Inactive, "Task::active");
        _;
    }

                else {
-                  // Un-confirm SC, mark task as inactive, mark allocated as false, mark lifecycle as None

-                  // Mark task as inactive by unapproving subcontractor.
-                  // As subcontractor can only be approved if task is allocated
-                  _unapproved = true;
-                  tasks[_taskID].unApprove();

+                  // Mark task as inactive, mark allocated as false, add to the allocation queue

+                  // Mark task as inactive
+                  tasks[_taskID].deActivate();

                    // Mark task as not allocated.
                    tasks[_taskID].unAllocateFunds();

                    // Reduce total allocation by old task cost.
                    // As as needs to go though funding process again.
                    totalAllocated -= _taskCost;

                    // Add this task to _changeOrderedTask array. These tasks will be allocated first.
                    _changeOrderedTask.push(_taskID);
                }

1. 1-step ownership change (non-critical)

One step process offers no protection for the cases when ownership transfer is performed mistakenly or with any malicious intent.

Adding a modest complexity of an additional step and a delay is a low price to pay for having time to evaluate the change.

Proof of Concept

HomeFi sets a new admin immediately:

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

HomeFi's admin:

    function replaceAdmin(address _newAdmin)
        external
        override
        onlyAdmin
        nonZero(_newAdmin)
        noChange(admin, _newAdmin)
    {
        // Replace admin
        admin = _newAdmin;

        emit AdminReplaced(_newAdmin);
    }

HomeFiProxy calls for the proxyAdmin ownership transfer immediately:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/HomeFiProxy.sol#L150-L157

    function changeProxyAdminOwner(address _newAdmin)
        external
        onlyOwner
        nonZero(_newAdmin)
    {
        // Transfer ownership to new admin.
        proxyAdmin.transferOwnership(_newAdmin);
    }

Recommended Mitigation Steps

Consider utilizing two-step ownership transferring process (proposition and acceptance in separate actions) with a noticeable delay between the steps to enforce the transparency and stability of the system

2. Lender fee is uncapped

Consider checking for the validity of the new fee (1-1000):

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/HomeFi.sol#L185-L197

    function replaceLenderFee(uint256 _newLenderFee)
        external
        override
        onlyAdmin
    {
        // Revert if no change in lender fee
        require(lenderFee != _newLenderFee, "HomeFi::!Change");

        // Reset variables
        lenderFee = _newLenderFee;

        emit LenderFeeReplaced(_newLenderFee);
    }

3. Comment is incomplete

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Community.sol#L164-L165

        // Emit event if _hash. This way this hash needs not be stored in memory.
        emit UpdateCommunityHash(_communityID, _hash);

Recommended Mitigation Steps

Update the comment, for example: // Emit event broadcasting new _hash. This way this hash needs not be stored in memory

4. Comment claims burn instead of mint

claimInterest() mints new _wrappedToken, not burns it:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Community.sol#L849-L850

            // Burn _interestEarned amount wrapped token to lender
            _wrappedToken.mint(_lender, _interestEarned);

Recommended Mitigation Steps

Should be mint as the debt record is increased

5. Typos in comments

To be Revertx2:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Project.sol#L514-L520

            // Revet if sender is not builder or contractor
            require(
                signer == builder || signer == contractor,
                "Project::!(GC||Builder)"
            );
        } else {
            // Revet if sender is not builder, contractor or task's subcontractor

To be // As it needs to go though:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Project.sol#L454-L456

                    // Reduce total allocation by old task cost.
                    // As as needs to go though funding process again.
                    totalAllocated -= _taskCost;

6. Incorrect function description

unApprove() description doesn't match the logic:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/libraries/Tasks.sol#L153-L164

    /**
     * @dev Set a task as un accepted/approved for SC

     * @dev modifier onlyActive

     * @param _self Task the task being set as funded
     */
    function unApprove(Task storage _self) internal {
        // State/ lifecycle //
        _self.alerts[uint256(Lifecycle.SCConfirmed)] = false;
        _self.state = TaskStatus.Inactive;
    }

7. raiseDispute doesn't check for signatory validity

Whenever _signature isn't valid there is no revert, but zero _signer will be put to disputes structure:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Disputes.sol#L84-L116

    function raiseDispute(bytes calldata _data, bytes calldata _signature)
        external
        override
        onlyProject
    {
        // Recover signer from signature
        address _signer = SignatureDecoder.recoverKey(
            keccak256(_data),
            _signature,
            0
        );

        // Decode params from _data
        (
            address _project,
            uint256 _taskID,
            uint8 _actionType,
            bytes memory _actionData,
            bytes memory _reason
        ) = abi.decode(_data, (address, uint256, uint8, bytes, bytes));

        // Revert if _actionType is invalid
        require(
            _actionType > 0 && _actionType <= uint8(ActionType.TaskPay),
            "Disputes::!ActionType"
        );

        // Store dispute details
        Dispute storage _dispute = disputes[disputeCount];
        _dispute.status = Status.Active;
        _dispute.project = _project;
        _dispute.taskID = _taskID;
        _dispute.raisedBy = _signer;

8. Project's changeOrder uses hard coded TaskAllocated constant

changeOrder() hard codes 1 as TaskAllocated enumeration item:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Project.sol#L422-L423

            // If tasks are already allocated with old cost.
            if (tasks[_taskID].alerts[1]) {

Recommended Mitigation Steps

Consider using the constant to improve readability and reduce operation error surface:

            // If tasks are already allocated with old cost.
-           if (tasks[_taskID].alerts[1]) {
+           if (tasks[_taskID].alerts[Lifecycle.TaskAllocated]) {

1. SignatureDecoder.recoverKey() is called twice by two raiseDispute functions with the same result

Disputes' raiseDispute() is called only by Project's raiseDispute() with _data passed over:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Project.sol#L492-L502

    /// @inheritdoc IProject
    function raiseDispute(bytes calldata _data, bytes calldata _signature)
        external
        override
    {
        // Recover the signer from the signature
        address signer = SignatureDecoder.recoverKey(
            keccak256(_data),
            _signature,
            0
        );

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Project.sol#L534-L536

        // Make a call to Disputes contract raiseDisputes.
        IDisputes(disputes).raiseDispute(_data, _signature);
    }

Disputes' raiseDispute() repeats SignatureDecoder.recoverKey(keccak256(_data),_signature, 0) with the same result:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Disputes.sol#L84-L94

    function raiseDispute(bytes calldata _data, bytes calldata _signature)
        external
        override
        onlyProject
    {
        // Recover signer from signature
        address _signer = SignatureDecoder.recoverKey(
            keccak256(_data),
            _signature,
            0
        );

Recommended Mitigation Steps

Consider introducing the signer argument and sending the _signer to the downstream raiseDispute():

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Project.sol#L534-L536

        // Make a call to Disputes contract raiseDisputes.
-       IDisputes(disputes).raiseDispute(_data, _signature);
+       IDisputes(disputes).raiseDispute(_data, _signature, _signer);
    }

2. Unnecessary _msgSender calls in Community's lendToProject

lendToProject() does three calls instead of one:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Community.sol#L379-L380

        // Local instance of variable. For saving gas.
        address _sender = _msgSender();

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Community.sol#L442-L446

        // Transfer _lenderFee to HomeFi treasury from lender account
        _currency.safeTransferFrom(_msgSender(), homeFi.treasury(), _lenderFee);

        // Transfer _amountToProject to _project from lender account
        _currency.safeTransferFrom(_msgSender(), _project, _amountToProject);

Recommended Mitigation Steps

As _msgSender() is already saved to memory consider using _sender variable instead of the call

3. Unnecessary _msgSender calls in Project's inviteSC

inviteSC() does two calls instead of one:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Project.sol#L300-L304

        // Revert if sender is neither builder nor contractor.
        require(
            _msgSender() == builder || _msgSender() == contractor,
            "Project::!Builder||!GC"
        );

Same for acceptInviteSC():

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Project.sol#L322-L324

        for (uint256 i = 0; i < _length; i++) {
            tasks[_taskList[i]].acceptInvitation(_msgSender());
        }

Recommended Mitigation Steps

Consider introducing and using _sender memory variable instead of the calls

4. Unnecessary storage update

lentAmount is updated even if not changed, when _repayAmount <= _interest:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Community.sol#L785-L802

        if (_repayAmount > _interest) {
            // If repayment amount is greater than interest then
            // set lent = lent + interest - repayment.
            // And set interest = 0.
            uint256 _lentAndInterest = _lentAmount + _interest;

            // Revert if repayment amount is greater than sum of lent and interest.
            require(_lentAndInterest >= _repayAmount, "Community::!Liquid");
            _interest = 0;
            _lentAmount = _lentAndInterest - _repayAmount;
        } else {
            // If repayment amount is lesser than interest, then set
            // interest = interest - repayment
            _interest -= _repayAmount;
        }

        // Update community  project details
        _communityProject.lentAmount = _lentAmount;

Recommended Mitigation Steps

Consider moving storage update to the part of logic where it happens:

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Community.sol#L785-L802

        if (_repayAmount > _interest) {
            // If repayment amount is greater than interest then
            // set lent = lent + interest - repayment.
            // And set interest = 0.
            uint256 _lentAndInterest = _lentAmount + _interest;

            // Revert if repayment amount is greater than sum of lent and interest.
            require(_lentAndInterest >= _repayAmount, "Community::!Liquid");
            _interest = 0;
            _lentAmount = _lentAndInterest - _repayAmount;
+           _communityProject.lentAmount = _lentAmount;
        } else {
            // If repayment amount is lesser than interest, then set
            // interest = interest - repayment
            _interest -= _repayAmount;
        }

        // Update community  project details
-       _communityProject.lentAmount = _lentAmount;