chaduke - LibUbiquityPool.collectRedemption() lacks modifier collateralEnabled(collateralIndex).

[sherlock-admin]

chaduke

medium

LibUbiquityPool.collectRedemption() lacks modifier collateralEnabled(collateralIndex).

Summary

LibUbiquityPool.collectRedemption() lacks modifier collateralEnabled(collateralIndex). As a result, even when a collateral is disabled, a user can still collected the redeemed collateral.

Vulnerability Detail

The redeeming of collateral takes two steps: 1. redeemDollar() and 2 collectRedemption().

These two steps cannot be be performed in the same block to avoid flashload manipulation.

While the redeemDollar() has the collateralEnabled(collateralIndex) modifier, the collectRedemption() does not.
As a result, if the collateral is disabled between the two steps to disable redeeming. The disnabling will fail since the second does not have the modifier collateralEnabled(collateralIndex). The user will still be able to complete the redemption process.

Impact

LibUbiquityPool.collectRedemption() lacks modifier collateralEnabled(collateralIndex). The admin/owner cannot stop the redeeming process of two steps once the first step is completed. Even a collateral is disabled, a user can still call LibUbiquityPool.collectRedemption() to collect the collateral. Stopping the collateral transfer will not succeed.

Code Snippet

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

Tool used

VScode

Manual Review

Recommendation

Add the modifier collateralEnabled(collateralIndex) to LibUbiquityPool.collectRedemption()

[sherlock-admin2]

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

auditsea commented:

It's protocol decision

[nevillehuang]

Invalid, in redeemDollar(), the collateralEnabled() is already performed., where in the the redeemCollateralBalances mapping for that particular collateral Index has been incremented ass seen here. Then there will be a check that this value is greather than zero in collectRedemption here.

If collateral is not enabled, this will result in no tokens being transferred to user since they cannot call redeemDollar() in the first place. For any pending redemptions, it is still users right to collect redemption, so no issue here.