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

nobody2018 - The currentMinted of vault will not decrease when the user repays TAU #32

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

nobody2018

medium

The currentMinted of vault will not decrease when the user repays TAU

Summary

When a user borrows TAU from a vault, the currentMinted[vault] will be accumulated and cannot exceed the mintLimit[vault], otherwise the user's borrowing fails. When user repays TAU to vault, currentMined[vault] does not decrease. If users borrow and repay frequently or the malicious user borrows and repays all the time, the value of currentMinted[vault] will soon be close to the value of mintLimit[vault]. Finally, it can only be recovered by manually updating mintLimit[vault].

Vulnerability Detail

The reason for this issue is that vault calls TAU.burnFrom when processing user repayment, which did not reduce the value of currentMinted [vault]. Let's look at the [code snippet](https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/TAU.sol#L71-L84):

//
    function burnFrom(address account, uint256 amount) public virtual override {
        super.burnFrom(account, amount);
        _decreaseCurrentMinted(account, 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];	//account=user, so currentMinted[account] always equal to 0.
        if (accountMinted >= amount) {	//never into if statement
            currentMinted[msg.sender] = accountMinted - amount;
        }
    }

BurnFrom internally calls _DecreaseCurrentMinted that the account parameter equal to user's address, so currentMinted[account] is always 0, and if statement's condition (0>=amount) will never be satisfied.

Impact

This issue will cause users to fail to borrow TAU from vault. The core function of the system fails.

Code Snippet

Tool used

Manual Review

Recommendation

--- a/taurus-contracts/contracts/TAU.sol
+++ b/taurus-contracts/contracts/TAU.sol
@@ -76,7 +76,7 @@ contract TAU is ERC20, ERC20Burnable {
     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];
+        uint256 accountMinted = currentMinted[msg.sender];
         if (accountMinted >= amount) {
             currentMinted[msg.sender] = accountMinted - amount;
         }
~
~

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