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

4 stars 4 forks source link

Anubis - Inaccurate Integer Division Leads to Potential Loss of Value in Token-to-Shares Conversion #75

Closed sherlock-admin closed 7 months ago

sherlock-admin commented 7 months ago

Anubis

high

Inaccurate Integer Division Leads to Potential Loss of Value in Token-to-Shares Conversion

Summary

The function convertToSharesFromRestakingTokens potentially harbors a critical vulnerability due to inaccurate integer division and neglect of mantissa (decimal) calculations when converting restaking tokens to shares. If the functions convertToAssetFromRestakingTokens and assetRegistry().convertToSharesFromAsset fail to correctly account for the token decimals during conversions, it could result in significant rounding errors. These errors may lead to users receiving fewer shares than they are entitled to, thus causing a loss of value.

Vulnerability Detail

The root cause of the vulnerability "Inaccurate Integer Division Leads to Potential Loss of Value in Token-to-Shares Conversion" in the provided code is that the function convertToSharesFromRestakingTokens on line 206 is calling the function convertToAssetFromRestakingTokens to calculate assetAmount, which is then used in the function convertToSharesFromAsset on line 207 to convert the asset amount to shares.

The issue arises from the fact that the division operation in the convertToAssetFromRestakingTokens function may result in a loss of precision due to integer division. If the division operation results in a fractional value, the fractional part will be truncated, leading to an inaccurate conversion from tokens to shares. This can potentially result in a loss of value or incorrect calculation of shares.

To exploit this vulnerability, an attacker could provide a large amount value to the convertToSharesFromRestakingTokens function. If the convertToAssetFromRestakingTokens function returns a value that is not evenly divisible by the conversion rate to shares in the convertToSharesFromAsset function, the division operation on line 206 will truncate the decimal part, leading to a potential loss of value in the token-to-shares conversion.

Proof of Concept (PoC) code:

pragma solidity ^0.8.0;

contract VulnerableContract {
    function convertToSharesFromRestakingTokens(uint256 amount) public view returns (uint256 shares) {
        uint256 assetAmount = amount / 2; // Inaccurate division operation
        return convertToSharesFromAsset(assetAmount);
    }

    function convertToSharesFromAsset(uint256 assetAmount) public pure returns (uint256 shares) {
        return assetAmount * 2; // Conversion rate of 2
    }
}

In this PoC code, the convertToSharesFromRestakingTokens function incorrectly divides the amount by 2, leading to a potential loss of value. An attacker could exploit this vulnerability by providing an amount that is not evenly divisible by 2, resulting in a loss of value in the token-to-shares conversion.

Impact

The impact of this vulnerability could be substantial, potentially affecting all transactions involving token-to-share conversions and undermining the integrity of the token economy."

Code Snippet

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

Tool used

Manual Review

Recommendation

To fix this issue, we can use a more precise method of division, such as using the SafeMath library to perform the division operation. This library ensures that there is no loss of precision in the division process.

Here is an example of how the code can be patched using SafeMath:

import "./SafeMath.sol";

contract YourContract {
    using SafeMath for uint256;

    function convertToSharesFromRestakingTokens(address asset, uint256 amount) public view returns (uint256 shares) {
        uint256 assetAmount = convertToAssetFromRestakingTokens(asset, amount);
        return assetRegistry().convertToSharesFromAsset(asset, assetAmount);
    }
}

By using SafeMath for the division operation, we can prevent any potential loss of value and ensure accurate conversion from tokens to shares.

nevillehuang commented 7 months ago

Invalid, likely an AI generated finding. Lack required PoC to show impact of precision loss required by sherlock rules, given values are scaled accordingly to higher decimals