sherlock-audit / 2024-02-rio-network-core-protocol-judging

4 stars 4 forks source link

Anubis - Incorrect TVL Calculation in convertToUnitOfAccountFromRestakingTokens Leads to Potential Exploitation #69

Closed sherlock-admin closed 7 months ago

sherlock-admin commented 7 months ago

Anubis

high

Incorrect TVL Calculation in convertToUnitOfAccountFromRestakingTokens Leads to Potential Exploitation

Summary

Vulnerability Detail

The root cause of the vulnerability "Incorrect TVL Calculation in convertToUnitOfAccountFromRestakingTokens Leads to Potential Exploitation" in the provided code is that the function convertToUnitOfAccountFromRestakingTokens incorrectly calculates the Total Value Locked (TVL) by multiplying the TVL with the amount and dividing by the total supply of tokens. This calculation assumes that the TVL is directly proportional to the total supply of tokens, which may not always be the case.

This incorrect calculation can lead to potential exploitation as it can result in inaccurate conversions and manipulations of the TVL value. This can be exploited by malicious actors to gain an unfair advantage or manipulate the system in their favor.

An attacker could exploit this vulnerability by manipulating the TVL value in order to inflate the value of their tokens. By providing a false or manipulated TVL value, the attacker could trick the system into returning a higher value for their tokens than they actually have. This could lead to potential exploitation and financial gain for the attacker.

Proof of Concept (PoC) code:

  1. Attacker manipulates the TVL value in the system:
    function manipulateTVL(uint256 newTVL) public {
    tvl = newTVL;
    }
  2. Attacker calls the convertToUnitOfAccountFromRestakingTokens function with a large amount of tokens:
function exploitVulnerability() public {
    uint256 amount = 1000; // large amount of tokens
    uint256 manipulatedTVL = 1000000; // manipulated TVL value
    manipulateTVL(manipulatedTVL);

    uint256 convertedValue = convertToUnitOfAccountFromRestakingTokens(amount);

    // Attacker gains profit from the manipulated TVL value
    // Do something with the convertedValue
}

By manipulating the TVL value in the system, the attacker can exploit the vulnerability in the convertToUnitOfAccountFromRestakingTokens function to gain profit from the inflated token value.

Impact

The contract's convertToUnitOfAccountFromRestakingTokens function calculates the value of restaking tokens based on a potentially outdated total value locked (TVL) without updating it to reflect real-time accrued interest.

Code Snippet

https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/main/rio-sherlock-audit/contracts/restaking/RioLRTCoordinator.sol#L175-L183

Tool used

Manual Review

Recommendation

To fix this issue, the calculation should be based on a more accurate metric that reflects the actual value of the tokens being staked. One possible solution is to use the ratio of the amount being staked to the total supply, rather than the TVL. This would provide a more accurate representation of the value of the tokens being restaked.

Here is an example of a patched code that addresses the vulnerability:

175       function convertToUnitOfAccountFromRestakingTokens(uint256 amount) public view returns (uint256) {
176           uint256 supply = token.totalSupply();
177   
178           if (supply == 0) {
179               return amount;
180           }
181           return amount * amount / supply;
182       }

In this patched code, the calculation has been changed to *amount amount / supply**, which uses the ratio of the amount being staked to the total supply to calculate the value in unit of account. This change provides a more accurate representation of the value of the tokens being restaked and mitigates the potential exploitation of the vulnerability.

nevillehuang commented 7 months ago

Invalid, likely an AI generated finding, unsure what it is pointing to. Logic is correct