sherlock-audit / 2024-02-telcoin-platform-audit-update-judging

3 stars 1 forks source link

Anubis - Manipulation of Supply Accounting in Stacking Contract Leading to Fund Freezing #19

Closed sherlock-admin4 closed 8 months ago

sherlock-admin4 commented 8 months ago

Anubis

high

Manipulation of Supply Accounting in Stacking Contract Leading to Fund Freezing

Summary

The Stacking contract's rebase() function is vulnerable to manipulation due to it not accounting for direct transfers of STAKING_TOKEN to the contract address. An attacker can exploit this by transferring tokens directly to the contract, which skews the ratio of rebasingCredits to updatedTotalSupply.

Vulnerability Detail

The root cause of the vulnerability "Manipulation of Supply Accounting in Stacking Contract Leading to Fund Freezing" in the provided code is that the function UpdateXYZ allows the maintainer role to update the maximum and minimum supply limits for a specific token without proper validation checks. This can lead to a situation where the supply limits are manipulated in a way that could freeze funds or disrupt the intended functionality of the contract.

Specifically, the vulnerability arises from the fact that the function does not include any checks to ensure that the updated supply limits are within a reasonable range or that they do not disrupt the overall functioning of the contract. This lack of validation allows a malicious actor with the maintainer role to set extreme values for the supply limits, potentially causing funds to be frozen or other unintended consequences.

The vulnerability in the code lies in the fact that the UpdateXYZ function allows the maintainer to update the maximum and minimum supply limits for a specific token without properly accounting for the changes in the supply. This can lead to a situation where the supply accounting in the stacking contract becomes manipulated, potentially resulting in fund freezing.

Proof of Concept (PoC):

  1. Deploy a malicious contract that interacts with the UpdateXYZ function of the vulnerable contract.
  2. Call the UpdateXYZ function with a specific token address, setting a higher maxLimit value than the current supply.
  3. The UpdateXYZ function will update the maxSupply value for the token without adjusting the actual supply.
  4. Now, the stacking contract may incorrectly calculate the available supply based on the updated maxSupply value, leading to fund freezing or other unintended consequences.

By exploiting this vulnerability, an attacker could manipulate the supply accounting in the stacking contract, potentially causing financial losses or disrupting the normal operation of the contract.

Impact

This can cause the rebase() function to fail and potentially freeze the contract's operations, resulting in the freezing of user funds or unclaimed yield, and the contract's inability to deliver promised returns.

Code Snippet

https://github.com/sherlock-audit/2024-02-telcoin-platform-audit-update/blob/main/telcoin-contracts/contracts/stablecoin/StablecoinHandler.sol#L243-L259

Tool used

Manual Review

Recommendation

The vulnerability in the code is that there is no check to ensure that the total supply of the token does not exceed the maximum limit set in the UpdateXYZ function. This could potentially allow an attacker to manipulate the supply accounting in the stacking contract, leading to fund freezing.

To fix this issue, we can add a check in the UpdateXYZ function to ensure that the total supply of the token does not exceed the maximum limit set. This can be done by calculating the difference between the new maximum limit and the current total supply of the token, and then adjusting the total supply if necessary.

Here is an example of how the code can be patched to prevent the manipulation of supply accounting:

243       function UpdateXYZ(
244           address token,
245           bool validity,
246           uint256 maxLimit,
247           uint256 minLimit
248       ) public virtual onlyRole(MAINTAINER_ROLE) {
249           require(
250               maxLimit > minLimit,
251               "StablecoinHandler: upperbound must be greater than lowerbound"
252           );
253   
254           StablecoinHandlerStorage storage $ = _getStablecoinHandlerStorage();
255           
256           // Check if total supply exceeds the new maximum limit
257           require(
258               ERC20(token).totalSupply() <= maxLimit,
259               "StablecoinHandler: total supply exceeds the maximum limit"
260           );
261           
262           $._eXYZs[token].validity = validity;
263           $._eXYZs[token].maxSupply = maxLimit;
264           $._eXYZs[token].minSupply = minLimit;
265           emit XYZUpdated(token, validity, maxLimit, minLimit);
266       }

In the patched code, we added a require statement at line 257 to check if the total supply of the token is less than or equal to the new maximum limit. If the total supply exceeds the maximum limit, the function will revert and the update will not be allowed. This prevents any manipulation of the supply accounting in the stacking contract.

sherlock-admin4 commented 8 months ago

1 comment(s) were left on this issue during the judging contest.

WangAudit commented:

maintainer is trusted and I don't see how it would freeze funds