LethL - Conflicting Mapping Keys in _decreaseCurrentMinted() Function Can Cause Infinite TAU Token Minting and Incorrect currentMinted Calculation

[sherlock-admin]

LethL

high

Conflicting Mapping Keys in _decreaseCurrentMinted() Function Can Cause Infinite TAU Token Minting and Incorrect currentMinted Calculation

Summary

In the TAU contract, the accountMinted and currentMinted[msg.sender] variables in the _decreaseCurrentMinted() function refer to the same mapping of currentMinted but accessed using different keys.
This can cause the given mint limit vault to repeatedly call the burnFrom() and mint() functions, thereby minting an infinite number of TAU tokens.
In addition, there is a possibility of incorrect calculation of currentMinted when the burnFrom() function is called.

Vulnerability Detail

For Example:

1. The governance calls the setMintLimit() function to set a mint limit of 100 for Alice's vault (mintLimit[Alice] = 100).
2. Alice calls mint() function to mint 100 TAU tokens to Alice's address (currentMinted[Alice] = 100).
3. Alice executes the burnFrom() function with an amount of zero to any address that has currentMinted = 0. This will also trigger the _decreaseCurrrentMinted() function.
4. At line 80, since both accountMinted and amount are zero, the currentMinted[msg.sender] value for Alice will also become zero (currentMinted[Alice] = 0).
5. Thus, Alice can call the mint() function again with a maximum mint limit of 100 TAU tokens.
6. Alice can repeatedly execute steps 2-5, allowing her to manipulate the currentMinted value and mint an infinite number of TAU tokens.

Impact

1. The given mint limit vault can arbitrarily mint the TAU tokens.
2. When the burnFrom() function is called, there is a possibility that the currentMinted value of msg.sender may be calculated incorrectly.

Code Snippet

    function mint(address recipient, uint256 amount) external {
        // Check whether mint amount exceeds mintLimit for msg.sender
        uint256 newMinted = currentMinted[msg.sender] + amount;
        if (newMinted > mintLimit[msg.sender]) {
            revert mintLimitExceeded(newMinted, mintLimit[msg.sender]);
        }

        // Update vault currentMinted
        currentMinted[msg.sender] = newMinted;

        // Mint TAU to recipient
        _mint(recipient, amount);
    }

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

    function burnFrom(address account, uint256 amount) public virtual override {
        super.burnFrom(account, amount);
        _decreaseCurrentMinted(account, amount);
    }

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

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

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

Tool used

Manual Review

Recommendation

Using totalSupply() from ERC20 instead of currentMinted because the TAU contract is unable to determine whether currentMinted[account] is the minter or not. Additionally, only the TAU contract has been granted permission to mint() TAU tokens.

Also, remove the burnFrom() function and use the burn() function instead since all the addresses used as parameters in the burnFrom() function throughout the entire contract refer to msg.sender.

Duplicate of #149