hats-finance / HATs-Arbitration-Contracts-0x79a618f675857b45934ca1c413fd5f409cf89735

MIT License
0 stars 1 forks source link

in HATToken, setMinter Function doesn't consider the _cap #40

Open hats-bug-reporter[bot] opened 1 year ago

hats-bug-reporter[bot] commented 1 year ago

Github username: @0xmahdirostami Submission hash (on-chain): 0x9e2f144ab26abe65d3aa2d2ac471d43fa659f0c20079712d0019014f650e5271 Severity: medium

Description: Description\ The HATToken, based on the ERC20Capped standard, has a predefined cap of 100,000,000e18 tokens. The owner of the token can authorize specific addresses to mint a certain amount. If anyone attempts to mint tokens, the function should check if the cap is exceeded. However, there is an issue in the setMinter function; it doesn't verify if the cap is surpassed. This means that users might have the permission to mint, but they won't be able to do so.

Impact\ The impact of this issue is that the owner may specify minting amounts for users, but if those amounts exceed the total cap, users won't be able to mint the full quantity they've been permitted.

Attachments

  1. Proof of Concept (PoC) File

    The setMinter function doesn't include a cap check:

    function setMinter(address _minter, uint256 _seedAmount) external onlyOwner {
        minters[_minter] = _seedAmount;
        emit MinterSet(_minter, _seedAmount);
    }
  2. Revised Code File (Optional)

    The suggested fix for this vulnerability is to add a cap check in the setMinter function. This ensures that minters can confidently mint the total amount they have been permitted.

0xmahdirostami commented 1 year ago

Revised Code:

+    uint256 public granted;

+    function setMinter(address _minter, uint256 _seedAmount) external onlyOwner {
+        granted = granted + _seedAmount - minters[_minter];
+        require(granted <= cap(), "cap is surpassed");
         minters[_minter] = _seedAmount;
         emit MinterSet(_minter, _seedAmount);
    }
bahurum commented 1 year ago

_seedAmount is just the maximum amount that could be minted by the minter. If that amount exceeds the cap, the minter will be able to mint up to the cap.

0xmahdirostami commented 1 year ago

_seedAmount is just the maximum amount that could be minted by the minter. If that amount exceeds the cap, the minter will be able to mint up to the cap.

@bahurum thanks for the feedback, Yes you are right, but it shouldn’t exceed the cap, for example, consider the following scenario:

In this scenario, the user has permission to mint, but he won't be able to do so. The cap check in setMinter function ensures that minters can confidently mint the total amount they have been permitted.

jellegerbrandy commented 1 year ago

The scenerio you describe is one where the user is misunderstand what a permission is. This is not different than giving, say, 1inch permission to spend an unlimited amount of my tokens - this does not imply that I have unlimited tokens