code423n4 commented 1 year ago

Lines of code

https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/plugins/assets/stargate/StargatePoolFiatCollateral.sol#L112

Vulnerability details

Impact

StargatePoolFiatCollateral.refPerTok() May return 0, resulting in a price of (0,0)

Proof of Concept

In StargatePoolFiatCollateral.refPerTok(), if _totalSupply == 0, rate = 0 is returned.

    function refPerTok() public view virtual override returns (uint192 _rate) {
        uint256 _totalSupply = pool.totalSupply();

        if (_totalSupply != 0) {
            _rate = divuu(pool.totalLiquidity(), _totalSupply);
        }

    }

If refPerTok() == 0 it causes tryPrice() to return (0,0).

(0, 0) is a valid price.

which may cause the price to be wrong, and may cause savedLowPrice to be wrong as well.

Tools Used

Recommended Mitigation Steps

If _totalSupply==0, return FIX_ONE.

    function refPerTok() public view virtual override returns (uint192 _rate) {
        uint256 _totalSupply = pool.totalSupply();

        if (_totalSupply != 0) {
            _rate = divuu(pool.totalLiquidity(), _totalSupply);
-       }
+       } else {
+           _rate = FIX_ONE;
+       }

    }

Assessed type

Context

c4-judge commented 1 year ago

thereksfour marked the issue as primary issue

tbrent commented 1 year ago

I think all pool tokens would need to be redeemed (not just those held by StargateRewardableWrapper) and therefore this feels more like low severity / QA. However the change is worth making.

c4-sponsor commented 1 year ago

tbrent marked the issue as disagree with severity

thereksfour commented 1 year ago

@bin2chen66 Please take a look

bin2chen66 commented 1 year ago

@thereksfour agree, it's ok