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

chaduke - _decreaseCurrentMinted() does not revise currentMinted() correctly, as a result, a wrong mint limit might be enforced on a vault. #120

Closed
sherlock-admin opened this issue Mar 13, 2023 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Mar 13, 2023

chaduke

medium

_decreaseCurrentMinted() does not revise currentMinted() correctly, as a result, a wrong mint limit might be enforced on a vault.

Summary

_decreaseCurrentMinted() does not revise currentMinted correctly, as a result, a wrong mint limit might be enforced on a vault.

Vulnerability Detail

The _decreaseCurrentMinted() function was used in burn() and burnFrom() functions to decrease the accounting of currentMinted .

https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/TAU.sol#L76-L83

However, the modification only occurs when accountMinted >= amount, which means, if accountMinted < amount, accountMinted will not be modified while TAU tokens are burned. This leads to inconsistency. As a result, when mint() is called to mint more TAU tokens by the vault, it might be reverted due to a false violation of mint limit.

Here is an example:

  1. Suppose Vault A has a mint limit of 2000, and Vault A has already minted 2000 TAU.

  2. Another user sent 100 TAU to Vault A.

  3. Vault A calls burn() to burn 2100, however, since 2100 > accountMinted[VaultA], accountMinted[VaultA] will not be modified, although in this case, accountMinted[VaultA] should be set to zero.

  4. Now if Vault A calls "mint()to mint 2000 TAU, it will be rejected due to accountMinted[VaultA] = 2000``. It will falsely report that Vault A already reaches the limit even though it has just burned 2100 TAU.

Impact

The accounting of accountMinted is incorrect for function _decreaseCurrentMinted(). As a result, a vault might not be able to mint more TAU even though it has not reached the limit - the system will falsely report that it has already reached the mint limit

Code Snippet

See above

Tool used

VScode

Manual Review

Recommendation

We will set accountMinted = 0 when accountMinted >= amount.

function _decreaseCurrentMinted(address account, uint256 amount) internal virtual {
        // If the burner is a vault, subtract burnt TAU from its currentMinted.
        // This has a few highly unimportant edge cases which can generally be rectified by increasing the relevant vault's mintLimit.
        uint256 accountMinted = currentMinted[account];
        if (accountMinted >= amount) {
            currentMinted[msg.sender] = accountMinted - amount;
        }
+     else{
+          currentMinted[msg.sender] = 0;
+     }
    }

Duplicate of #149

@github-actions github-actions bot added Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Mar 21, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Apr 1, 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 Medium A valid Medium severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

1 participant