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

HonorLt - Mint limit is not reduced when the Vault is burning TAU #149

Open
sherlock-admin opened this issue Mar 13, 2023 · 3 comments
Open
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin
Copy link
Contributor

HonorLt

medium

Mint limit is not reduced when the Vault is burning TAU

Summary

Upon burning TAU, it incorrectly updates the currentMinted when Vault is acting on behalf of users.

Vulnerability Detail

When the burn of TAU is performed, it calls _decreaseCurrentMinted to reduce the limit of tokens minted by the Vault:

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

The issue is that it subtracts accountMinted (which is currentMinted[account]) from currentMinted[msg.sender]. When the vault is burning tokens on behalf of the user, the account != msg.sender meaning the currentMinted[account] is 0, and thus the currentMinted of Vault will be reduced by 0 making it pretty useless.

Another issue is that users can transfer their TAU between accounts, and then amount > accountMinted will not be triggered.

Impact

currentMinted is incorrectly decreased upon burning so vaults do not get more space to mint new tokens.

Code Snippet

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

Tool used

Manual Review

Recommendation

A simple solution would be to:

     uint256 accountMinted = currentMinted[msg.sender];

But I suggest revisiting and rethinking this function altogether.

@github-actions github-actions bot added Medium A valid Medium severity issue Has Duplicates A valid issue with 1+ other issues describing the same vulnerability labels Mar 21, 2023
This was referenced Mar 21, 2023
@Sierraescape Sierraescape added Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed labels Mar 21, 2023
@Sierraescape
Copy link

@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Apr 1, 2023
@MLON33
Copy link

MLON33 commented Apr 10, 2023

@IAm0x52
Copy link
Collaborator

IAm0x52 commented Apr 18, 2023

Fix looks good. _decreaseCurrentMinted now always uses msg.sender as account

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

4 participants