StabilityPool Manual Review Findings

SPL-01M: Inexistent Normalization of Asset

Type	Severity	Location
Logical Fault		StabilityPool.sol:L801

Description:

The StabilityPool::_sendGainsToDepositor function will not attempt to normalize the amount value when transferring the asset in contrast to the rest of the codebase.

Impact:

Presently, the code will misbehave if non-18 decimal assets are introduced to AdminContract which is permitted and actually expected by some of the contracts in the system. If it is a business requirement to support unwrapped non-18 decimal assets, this finding will be upgraded in severity to "major".

Example:

Copy
787function _sendGainsToDepositor(
788    address _to,
789    address[] memory assets,
790    uint256[] memory amounts
791) internal {
792    uint256 assetsLen = assets.length;
793    require(assetsLen == amounts.length, "StabilityPool: Length mismatch");
794    for (uint256 i = 0; i < assetsLen; ++i) {
795        uint256 amount = amounts[i];
796        if (amount == 0) {
797            continue;
798        }
799        address asset = assets[i];
800        // Assumes we're internally working only with the wrapped version of ERC20 tokens
801        IERC20Upgradeable(asset).safeTransferFrom(address(this), _to, amount);
802    }
803    totalColl.amounts = _leftSubColls(totalColl, assets, amounts);
804
805    // Reset pendingCollGains since those were all sent to the borrower
806    Colls memory tempPendingCollGains;
807    pendingCollGains[_to] = tempPendingCollGains;
808}

Recommendation:

We advise the code to be streamlined, either normalizing the amount in StabilityPool::_sendGainsToDepositor or ensuring that only wrapped assets are introduced to the AdminContract::addNewCollateral function by evaluating their decimals.

Alleviation:

The decimals of newly introduced assets via AdminContract::addNewCollateral are now mandated to be equal to DEFAULT_DECIMALS, streamlining the codebase and thus alleviating this exhibit as a result.