sherlock-audit / 2023-02-openq-judging

4 stars 0 forks source link

Avci - in BountyCore:refundDeposit there is problem in logic of checking in function that checks wrong. #565

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

Avci

high

in BountyCore:refundDeposit there is problem in logic of checking in function that checks wrong.

Summary

the caller check is not correct

Vulnerability Detail

it's designed to not allow the caller who is not funded but it's checking wrongly with the address instead of msg sender who is actually funded

Impact

the bad actors who is actually not should able to call this will able to call and its against of what the project wants and will make problems

Code Snippet

  function refundDeposit(
        bytes32 _depositId,
        address _funder,
        uint256 _volume
    ) external virtual onlyDepositManager nonReentrant {
        require(!refunded[_depositId], Errors.DEPOSIT_ALREADY_REFUNDED);
        require(funder[_depositId] == _funder, Errors.CALLER_NOT_FUNDER);
        require(
            block.timestamp >= depositTime[_depositId] + expiration[_depositId],
            Errors.PREMATURE_REFUND_REQUEST
        );

        refunded[_depositId] = true;

        if (tokenAddress[_depositId] == address(0)) {
            _transferProtocolToken(funder[_depositId], _volume);
        } else if (isNFT[_depositId]) {
            _transferNft(
                tokenAddress[_depositId],
                funder[_depositId],
                tokenId[_depositId]
            );
        } else {
            _transferERC20(
                tokenAddress[_depositId],
                funder[_depositId],
                _volume
            );
        }
    }

Tool used

Manual Review

Recommendation

consider checking with msg.sender and who actually funded

IAm0x52 commented 1 year ago

https://github.com/sherlock-audit/2023-02-openq/blob/ba7f35654d6fa7637ef0f6db346e851fb978cde2/contracts/DepositManager/Implementations/DepositManagerV1.sol#L158-L161

Invalid. BountyCore#refundDeposit is only callable through the deposit manager which means only funder can refund.