StErMi - Ongoing bounties can receive NFT, but claimant will never receive them

[github-actions]

StErMi

medium

Ongoing bounties can receive NFT, but claimant will never receive them

Summary

Ongoing bounties can receive NFTs as funds, but the claimant will not be able to receive once they claim a bounty because it's not implemented in the claim process.

Vulnerability Detail

The Ongoing bounty contract implements the receiveNft function that allows a funder to fund the contract with an NFT token. When the claimant will claim the bounty, he/she will only be able to receive ERC20/ETH tokens but no the NFTs.

In the edge case where the bounty is only funded with NFT the claimant will get no rewards back.

Impact

Even if the bounty is funded with NFT the claimant will not be able to get any of them

Code Snippet

Ongoing Bounty implementation that can receive NFTs OngoingBountyV1.sol#L133-L160

function receiveNft(
    address _sender,
    address _tokenAddress,
    uint256 _tokenId,
    uint256 _expiration,
    bytes calldata
) external onlyDepositManager nonReentrant returns (bytes32) {
    require(
        nftDeposits.length < nftDepositLimit,
        Errors.NFT_DEPOSIT_LIMIT_REACHED
    );
    require(_expiration > 0, Errors.EXPIRATION_NOT_GREATER_THAN_ZERO);
    _receiveNft(_tokenAddress, _sender, _tokenId);

    bytes32 depositId = _generateDepositId();

    funder[depositId] = _sender;
    tokenAddress[depositId] = _tokenAddress;
    depositTime[depositId] = block.timestamp;
    tokenId[depositId] = _tokenId;
    expiration[depositId] = _expiration;
    isNFT[depositId] = true;

    deposits.push(depositId);
    nftDeposits.push(depositId);

    return depositId;
}

Claim process on Claim Manager contract ClaimManagerV1.sol#L173-L197

function _claimOngoingBounty(
    IOngoingBounty _bounty,
    address _closer,
    bytes calldata _closerData
) internal {
    _eligibleToClaimOngoingBounty(_bounty, _closer, _closerData);

    (address tokenAddress, uint256 volume) = _bounty.claimOngoingPayout(
        _closer,
        _closerData
    );

    emit TokenBalanceClaimed(
        _bounty.bountyId(),
        address(_bounty),
        _bounty.organization(),
        _closer,
        block.timestamp,
        tokenAddress,
        volume,
        _bounty.bountyType(),
        _closerData,
        VERSION_1
    );
}

Claim process on Ongoing Bounty contract. Only support ERC20/ETH transfers via _transferToken OngoingBountyV1.sol#L96-L112

function claimOngoingPayout(
    address _payoutAddress,
    bytes calldata _closerData
) external onlyClaimManager nonReentrant returns (address, uint256) {
    (, string memory claimant, , string memory claimantAsset) = abi.decode(
        _closerData,
        (address, string, address, string)
    );

    // @audit - user cannot claim more than one bounty?
    // @audit - what is `claimantAsset`? like the PR/issue ID?
    bytes32 _claimId = generateClaimId(claimant, claimantAsset);

    claimId[_claimId] = true;
    claimIds.push(_claimId);

    _transferToken(payoutTokenAddress, payoutVolume, _payoutAddress);
    return (payoutTokenAddress, payoutVolume);
}

Tool used

Manual Review

Recommendation

If the Ongoing bounty type does not support NFT (because it's an ongoing bounty) remove the receiveNft function or simply make it revert with an explanation message

Duplicate of #261