Stale risk fund assets may make protocol loose funds

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/RiskFund/RiskFund.sol#L152 https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/Shortfall/Shortfall.sol#L381

Vulnerability details

Vulnerability Details

When swapping Risk funds in a pool swapPoolsAssets(address[],uint256[],address[][]) from one market underlying asset type to convertibleBaseAsset , only a limited selected markets are supplied as input parameter.

function swapPoolsAssets(
        address[] calldata markets,
        uint256[] calldata amountsOutMin,
        address[][] calldata paths
    ) external override returns (uint256) {
...

But when auctioning off pool bad debts, every market’s bad debt in the pools are calculated to form poolBadDebt which is a total of all bad debts in the pool. Additionally, when auctioning off bad debt in Shortfall contract, there is no check for pool reserves staleness:

function _startAuction(address comptroller) internal {
        ...
        uint256 riskFundBalance = riskFund.poolReserves(comptroller); // @audit pool reserves may be stale
        uint256 remainingRiskFundBalance = riskFundBalance;
        uint256 incentivizedRiskFundBalance = poolBadDebt + ((poolBadDebt * incentiveBps) / MAX_BPS);
        if (incentivizedRiskFundBalance >= riskFundBalance) {
            auction.startBidBps =
                (MAX_BPS * MAX_BPS * remainingRiskFundBalance) /
                (poolBadDebt * (MAX_BPS + incentiveBps));
            remainingRiskFundBalance = 0;
            auction.auctionType = AuctionType.LARGE_POOL_DEBT;
        } else {
            uint256 maxSeizeableRiskFundBalance = incentivizedRiskFundBalance;

            remainingRiskFundBalance = remainingRiskFundBalance - maxSeizeableRiskFundBalance;
            auction.auctionType = AuctionType.LARGE_RISK_FUND;
            auction.startBidBps = MAX_BPS;
        }

This makes every auction using more or less stale pool reserves, which basically means looses for the protocol.

Impact

1. Auctions are improperly priced as funds available in the RiskFund to a pool is what determines if an asset is considered a LARGE_POOL_DEBT or LARGE_RISK_FUND.
2. Protocol is loosing value due to auctioning off the debt for fraction of price, when reserves can be used.

Proof of Concept

1. Pool Apha has three markets only of combined 200M TVL, with a bad debt of 10M, the bad debt is spread across the three markets A,B and C respectively, 5M, 3M, 2M.
2. The reserves from market A,B and C are 2M, 2M and 7M respectively (exclusive of protocol reserves).
3. An authorized user calls swapPoolsAssets(address[],uint256[],address[][]) with market A and B as items in markets input. The Reserve is successfully calculated and converted with a pool reserve of 4M for the Risk fund to use to auction off bad debt on Pool Alpha.
4. A week passes. An end user calls the startAuction(address) function on shortfall contract using Pool Alpha as the input param, and it calculates a total bad debts of pool Alpha to be 10M.
5. Now since the bad debt as calculated in shortfall is 10M for pool alpha and the reserves in risk fund for pool Alpha is 4M, it therefore means that this auction will be auctioned off as a LARGE_POOL_DEBT type.

Tools Used

Manual analysis

Recommended Mitigation Steps

All assets reserved from each market in a pool should be used when swapping to convertibleBaseAsset, and validation should check that all markets in the pool is available in the markets input. So also meaning a comptroller address input should be passed as input to check against the markets own comptroller address to make sure they of all same pool in swapPoolsAssets(address[],uint256[],address[][]). Additionally, please consider adding stalessness check.

Assessed type

Other

[c4-sponsor]

chechu marked the issue as sponsor acknowledged

[chechu]

We can start auctions multiple times if there is a bad debt so even if there is staleness or all assets aren’t swapped then also we can cover the bad debt later

[0xean]

@chechu - I am not sure I am 100% following your response here. You acknowledge its an issue, but it sounds like you believe that the protocol can simply start another auction.

Protocol is loosing value due to auctioning off the debt for fraction of price, when reserves can be used.

[c4-sponsor]

chechu marked the issue as sponsor disputed

[chechu]

"Dispute validity" is more appropriate. The key point here is that in the auctions of the Shortfall we are not "selling" the bad debt, we are really "selling" the available risk fund (with a discount), accepting tokens of bad debt that we'll use to reduce it. In the example provided by the warden:

• bad debt: $5M, $3M, $2M. Total: $10M
• risk fund: $2M, $2M, $7M, but the actually available for the auction is only $4M

So, we're going to sell $4M, accept tokens of A, B, and C to cover the bad debt and start the auction with an initial bid of 360 bps ($3.6M in terms of bad debt). A potential winner could place a bid of 380 bps, for example. So, the winner sent to the Shortfall contract these tokens:

• A: $1.9M (5 * 0.38)
• B: $1.14M (3 * 0.38)
• C: $0.76M (2 * 0.38)

The bad debt is reduced to $7.2M (10 - 3.8).

And the winner will receive $4M (from the risk fund, in USDT or generally in RiskFund.convertibalBaseAsset tokens).

After this, there won't be an economic incentive to start a new auction, because there isn't available risk fund to "buy" with discount. Bidders will have to wait until someone authorized swaps the reserves in the RiskFund, getting convertibleBaseAsset tokens. At that moment, a new auction can be started, and the remaining $7.2M could be covered (or not yet, but that is not a problem).

In this model, the authorized accounts decide when they want to make available the risk fund for the auctions. That is the expected behavior.

[0xean]

Thank you for the additional context, closing.

[c4-judge]

0xean marked the issue as unsatisfactory: Invalid