sherlock-audit / 2023-02-openq-judging

4 stars 0 forks source link

ck - `_claimOngoingBounty` only claims the `payoutToken` but allows NFT deposits #487

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

ck

medium

_claimOngoingBounty only claims the payoutToken but allows NFT deposits

Summary

_claimOngoingBounty only claims the payoutToken but allows NFT deposits

Vulnerability Detail

The _claimOngoingBounty only claims payoutToken by calling claimOngoingPayout:

    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)
        );

        bytes32 _claimId = generateClaimId(claimant, claimantAsset);

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

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

This is despite that fact that the bounty can also be funded by NFT deposits. The OngoingBountyV1 currently implements the receiveNFTfunction but fails to provide a way to claim them.

Impact

While NFT deposits can be recovered later, this breaks the protocol and temporarily locks the funders NFTs until the expiration period elapses. Users will also be deceived into assuming that the NFTs are part of the bounty.

Code Snippet

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/OngoingBountyV1.sol#L96-L112

Tool used

Manual Review

Recommendation

Prevent deposits of any other token other than the payoutToken for the case of ongoing bountys. Alternatively add claim functionality for NFTs.

Duplicate of #352