sherlock-audit / 2023-12-ubiquity-judging

2 stars 2 forks source link

fugazzi - Collect redemption doesn't check if the collateral is enabled #214

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 6 months ago

fugazzi

medium

Collect redemption doesn't check if the collateral is enabled

Summary

A missing check would allow users to call collectRedemption() for disabled collaterals.

Vulnerability Detail

Unlike mintDollar() and redeemDollar(), the implementation of collectRedemption() lacks the collateralEnabled(collateralIndex) modifier, that actually checks that the collateral given by collateralIndex is enabled.

The implementation does check if redemptions are paused (isRedeemPaused) but fails to check if the collateral is enabled isCollateralEnabled. The enable/disable configuration should supersede any mint/redeem pause setting. For example, governance may shutdown the collateral in an emergency situation by calling toggleCollateral() without actually disabling the individual pause configurations, with the intention of completely halting the collateral.

Impact

The collectRedemption() function fails to check if the , and would still allow to withdraw tokens even if the collateral is disabled.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L476-L478

Tool used

Manual Review

Recommendation

Add the collateralEnabled modifier to collectRedemption().

    function collectRedemption(
        uint256 collateralIndex
-   ) internal returns (uint256 collateralAmount) {
+   ) internal collateralEnabled(collateralIndex) returns (uint256 collateralAmount) {

Duplicate of #23

sherlock-admin2 commented 6 months ago

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

Since users' Ubiquity Dollar tokens are already burnt, they should be able to claim their underlying tokens even it's paused

sherlock-admin2 commented 6 months ago

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

Since users' Ubiquity Dollar tokens are already burnt, they should be able to claim their underlying tokens even it's paused