code-423n4 / 2023-09-venus-findings

4 stars 4 forks source link

The hardcoded value of MAX_DISTRIBUTION_SPEED makes impossible the use of low valued tokens like SHIB #456

Open c4-submissions opened 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/PrimeLiquidityProvider.sol#L310-L326

Vulnerability details

Impact

In contract PrimeLiquidityProvider, constant MAX_DISTRIBUTION_SPEED is set to 1e18. This constant serves the purpose of limiting the distribution speed of tokens, this is the number of tokens that can be accrued per block.

    function _setTokenDistributionSpeed(address token_, uint256 distributionSpeed_) internal {
        if (distributionSpeed_ > MAX_DISTRIBUTION_SPEED) {
            revert InvalidDistributionSpeed(distributionSpeed_, MAX_DISTRIBUTION_SPEED);
        }

        if (tokenDistributionSpeeds[token_] != distributionSpeed_) {
            // Distribution speed updated so let's update distribution state to ensure that
            //  1. Token accrued properly for the old speed, and
            //  2. Token accrued at the new speed starts after this block.
            accrueTokens(token_);

            // Update speed
            tokenDistributionSpeeds[token_] = distributionSpeed_;

            emit TokenDistributionSpeedUpdated(token_, distributionSpeed_);
        }
    }

This makes impossible the use of low valued tokens such as SHIB.

Proof of Concept

SHIB is a populat ERC20 token that has 18 decimals and a current value of 0.00000723 USD. This means that, using a distribution speed equal to MAX_DISTRIBUTION_SPEED, the total value accrued after 1,000,000 blocks would be of 7.23 USD, which makes impractical the use of such a token.

Tools Used

Manual review.

Recommended Mitigation Steps

Don't make the distribution speed cap a constant value, make it editable by governance.

Another option would be to create a mapping to store the distribution speed per initialized token. This would be more flexible but also more gas intensive.

Assessed type

Other

c4-pre-sort commented 1 year ago

0xRobocop marked the issue as duplicate of #414

c4-judge commented 11 months ago

fatherGoose1 marked the issue as unsatisfactory: Invalid

neumoxx commented 11 months ago

My report is marked as duplicate of #414 but here I talk about the practical impossibility to use certain tokens such as SHIB, not any problem related with token decimals (as stated in #414). The use of a hardcoded value of MAX_DISTRIBUTION_SPEED makes impossible the use of SHIB and many other tokens, and it cannot be solved by setting the distribution speed to any value (as you suggest here, because it is always capped by the max.

c4-judge commented 11 months ago

fatherGoose1 marked the issue as not a duplicate

fatherGoose1 commented 11 months ago

Agree this is not a duplicate. I find this to be QA. This issue only fits a very small selection of tokens. SHIB isn't present in Venus's current markets.

c4-judge commented 11 months ago

fatherGoose1 changed the severity to QA (Quality Assurance)

c4-judge commented 11 months ago

fatherGoose1 marked the issue as grade-b

neumoxx commented 11 months ago

Agree this is not a duplicate. I find this to be QA. This issue only fits a very small selection of tokens. SHIB isn't present in Venus's current markets.

SHIB is not, but for instance BTT is. You can see here https://www.coingecko.com/en/coins/bittorrent, its value is even lower that SHIB, and has also 18 decimals. BTT is one of the tokens used in Venus here https://app.venus.io/#/isolated-pools/pool/0x23b4404E4E5eC5FF5a6FFb70B7d14E3FabF237B0.

So please, consider medium severity here, given at least one token used by Venus could suffer the same impact as my SHIB example here.

fatherGoose1 commented 11 months ago

@neumoxx you have referenced an isolated pool, while only core pool tokens are a part of Prime.

"Interest reserves (part of the protocol income) from core pool markets are sent to the PSR (Protocol Share Reserve) contract."

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/README.md#income-collection-and-distribution

neumoxx commented 11 months ago

Sorry then, my bad. This report, then, can only show a future issue in case a token such as SHIB or BTT are added to the core pool.

Thanks for your work @fatherGoose1