No way to part ways with project contractor

[code423n4]

Lines of code

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

Vulnerability details

Impact

The function Project.inviteContractor adds a contractor to the project. Once a contractor is set and contractorConfirmed is set to true, the contractor address can not be changed anymore.

If there are any real-world disputes between the project builder and the contractor, there is no way to part ways and define a new contractor. The project could then be stalled due to the contractor not signing multisig messages and griefing the system. Funds in the project contract could be locked as recovering the currency tokens does not work due to tasks being unfinished.

Proof of Concept

Project.sol#L123

function inviteContractor(bytes calldata _data, bytes calldata _signature)
    external
    override
{
    // Revert if contractor has already confirmed his invitation
    require(!contractorConfirmed, "Project::GC accepted"); // @audit-info If a contractor is confirmed, the contractor can not be changed anymore

    // Decode params from _data
    (address _contractor, address _projectAddress) = abi.decode(
        _data,
        (address, address)
    );

    // Revert if decoded project address does not match this contract. Indicating incorrect _data.
    require(_projectAddress == address(this), "Project::!projectAddress");

    // Revert if contractor address is invalid.
    require(_contractor != address(0), "Project::0 address");

    // Store new contractor
    contractor = _contractor;
    contractorConfirmed = true;

    // Check signature for builder and contractor
    checkSignature(_data, _signature);

    emit ContractorInvited(contractor);
}

Project.recoverTokens

function recoverTokens(address _tokenAddress) external override {
    /* If the token address is same as currency of this project,
        then first check if all tasks are complete */
    if (_tokenAddress == address(currency)) {
        // Iterate for each task and check if it is complete.
        uint256 _length = taskCount;
        for (uint256 _taskID = 1; _taskID <= _length; _taskID++) {
            require(tasks[_taskID].getState() == 3, "Project::!Complete");
        }
    }

    // Create token instance.
    IDebtToken _token = IDebtToken(_tokenAddress);

    // Check the balance of _token in this contract.
    uint256 _leftOutTokens = _token.balanceOf(address(this));

    // If balance is present then it to the builder.
    if (_leftOutTokens > 0) {
        _token.safeTransfer(builder, _leftOutTokens);
    }
}

Project.recoverTokens reverts if currency tokens are to be rescued as long as the project has unfinished tasks. As a task can only be set as completed by calling Project.setComplete, which in turn needs the message to be signed by the contractor, the contractor is able to grief the project.

Tools Used

Manual review

Recommended mitigation steps

Consider implementing a function that allows resetting the current contractor and then allows inviting a new contractor.