Imbalanced Basket Weights Due to Changing Collateral Status

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/mixins/BasketLib.sol#L232

Vulnerability details

Vulnerability Details

The nextBasket function in the BasketLibP1 contract is responsible for selecting and weighting collateral tokens for a basket. It handles both primary and backup collateral, with the backup collateral being used when primary collateral is insufficient or unavailable.

The function may produce imbalanced baskets or fail to create a valid basket when backup collateral tokens become unusable during the basket creation process. This is particularly problematic because the status of collateral can change between the initial counting of good collateral and the assignment of weights.

Details

1. The function first counts the number of good backup collateral tokens.
2. It then attempts to assign weights to these tokens in a separate loop.
3. If a token's status changes to unusable between these two steps, the function will still expect to assign all counted weights, leading to potential imbalances.

Code Snippet

uint256 size = 0; // backup basket size
BackupConfig storage backup = config.backups[_targetName];

// Count good backup collateral
for (uint256 j = 0; j < backup.erc20s.length && size < backup.max; ++j) {
    if (goodCollateral(_targetName, backup.erc20s[j], assetRegistry)) size++;
}

if (size == 0) return false;

// Assign weights
uint256 assigned = 0;
for (uint256 j = 0; j < backup.erc20s.length && assigned < size; ++j) {
    if (goodCollateral(_targetName, backup.erc20s[j], assetRegistry)) {
        // ... assign weight ...
        assigned++;
    }
}
// No check here to ensure all weights were assigned

Impact

1. Basket imbalances: The function may create baskets with incorrect weight distributions.
2. Increased vulnerability: The system becomes more susceptible to rapid changes in collateral status.
3. Potential system instability: Imbalanced baskets could lead to broader system issues.
4. False positives: The function may indicate success even when it fails to assign all weights.

Scenario

Assume a basket needs 3 backup tokens. Initially, 3 tokens are good, but one becomes unusable before weight assignment. The function will try to distribute weight for 3 tokens but only assign to 2, leaving the basket imbalanced.

Fix

Implement a check after weight assignment to ensure all expected weights were assigned:

uint256 size = 0;
// ... (existing code to count good collateral) ...

uint256 assigned = 0;
// ... (existing code to assign weights) ...

// Add this check after the assignment loop
if (assigned < size) return false;

Assessed type

Context