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

Chinmay - modifyPositions functionality may unnecessarily fail #194

Closed
sherlock-admin opened this issue Mar 13, 2023 · 0 comments
Closed
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

Chinmay

medium

modifyPositions functionality may unnecessarily fail

Summary

The decreaseCurrentMInted function of TAU.sol doesn't work correctly. Hence the token minted number is never reduced when they are burned.

Vulnerability Detail

Several of the logic branches in modifyPositions requires TAU to be burned or minted. However minting is not possible if the set Mint Limit is crossed. While mint limit can be increased by the governance, this is a concern because on burning the tokens currentMinted value is incorrectly updated.

When we call burnFrom at Line 300 in BaseVault.sol the account parameter is actually the msg.sender of the call. THe burnFrom function calls decreaseCurrentMinted function with the same account parameter it was supplied with, and then it tries to reduce currentMinted value of this "account" which is actually the msg.sender who was trying to modify his position.

When we try to set mint limit, we are limiting mints to a vault address, and this should be consistent while burning too.

Impact

The above call flow explains that the currentMinted value for the vault never actually gets decreased it only keeps increasing. Thus minted tokens may cross the mint limit set for the vault and halt further execution including modifyPositions function. While the mint limit can be changed by governance but until the governance comes to know about this, the user is griefed and couldn't withdraw his collateral when he wanted to.

Code Snippet

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

Tool used

Manual Review

Recommendation

The line L#79 should be changed to accountMinted = currentMinted[msg.sender] because msg.sender here is the vault address.

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