Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

GimelSec - bountyIsClaimable() should revert when the bounty is invalid #500

Closed
github-actions bot opened this issue Feb 22, 2023 · 0 comments
Closed
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Non-Reward This issue will not receive a payout

Comments

@github-actions
Copy link

github-actions bot commented Feb 22, 2023

GimelSec

medium

bountyIsClaimable() should revert when the bounty is invalid

Summary

bountyIsClaimable() should revert when the bounty is invalid.

Vulnerability Detail

bountyIsClaimable() checks if bounty associated with _bountyAddress is open:

    /// @notice Checks if bounty associated with _bountyId is open
    /// @return bool True if _bountyId is associated with an open bounty
    function bountyIsClaimable(address _bountyAddress)
        public
        view
        returns (bool)
    {
        IBounty bounty = IBounty(payable(_bountyAddress));

        uint256 status = bounty.status();
        uint256 _bountyType = bounty.bountyType();

        if (
            _bountyType == OpenQDefinitions.ATOMIC ||
            _bountyType == OpenQDefinitions.ONGOING ||
            _bountyType == OpenQDefinitions.TIERED_PERCENTAGE ||
            _bountyType == OpenQDefinitions.TIERED_FIXED
        ) {
            return status == 0;
        } else {
            return status == 1;
        }
    }

A valid bounty should be minted by OpenQV1. But bountyIsClaimable() doesn't check if the _bountyAddress exists in OpenQV1.bountyIdToAddress. It means that users may get an invalid result from a malicious bounty contract which is not created from OpenQV1.

Furthermore, a valid bounty should have a valid bounty type [ATOMIC, ONGOING, TIERED_PERCENTAGE, TIERED_FIXED], else the bountyIsClaimable() should always return false or revert the transaction.

Impact

Users will get a wrong result from bountyIsClaimable(), users may also suffer from phishing attacks due to a malicious bounty contract.

Code Snippet

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/ClaimManager/Implementations/ClaimManagerV1.sol#L345

Tool used

Manual Review

Recommendation

Check OpenQV1.bountyIdToAddress and revert in invalid bounty type.

+   /// @notice Checks if bounty associated with _bountyAddress is open
+   /// @return bool True if _bountyAddress is associated with an open bounty
    function bountyIsClaimable(address _bountyAddress)
        public
        view
        returns (bool)
    {
        IBounty bounty = IBounty(payable(_bountyAddress));
+       require(openQ.bountyIdToAddress(bounty.bountyId()) == _bountyAddress, "Invalid bounty");

        uint256 status = bounty.status();
        uint256 _bountyType = bounty.bountyType();

        if (
            _bountyType == OpenQDefinitions.ATOMIC ||
            _bountyType == OpenQDefinitions.ONGOING ||
            _bountyType == OpenQDefinitions.TIERED_PERCENTAGE ||
            _bountyType == OpenQDefinitions.TIERED_FIXED
        ) {
            return status == 0;
        } else {
+           revert();
        }
    }

Duplicate of #386

@github-actions github-actions bot added Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue labels Feb 22, 2023
@sherlock-admin sherlock-admin added Non-Reward This issue will not receive a payout and removed Medium A valid Medium severity issue labels Mar 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Non-Reward This issue will not receive a payout
Projects
None yet
Development

No branches or pull requests

1 participant