sherlock-audit / 2023-01-uxd-judging

3 stars 1 forks source link

joestakey - `setRedeemable()` can lead to users unable to redeem. #405

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

joestakey

medium

setRedeemable() can lead to users unable to redeem.

Summary

If redeemable is changed after users have minted UDX, these users will be unable to redeem.

Vulnerability Detail

setRedeemable() allows the governor to change the redeemable token address. If a new redeemable token address is set, users who have previously minted UDX will not be able to redeem, as the following code block in _redeem() will revert:

319:         if(redeemable.allowance(msg.sender, address(this)) < redeemParams.amountToRedeem) {
320:             revert CtrlNotApproved(address(redeemable), msg.sender, redeemParams.amountToRedeem);
321:         }

Given that the user's balance of this new redeemable token is 0 (they own the previous redeemable token)

Impact

Users holding UDX are unable to redeem it.

Code Snippet

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/core/UXDController.sol#L134-L139

Tool used

Manual Review

Recommendation

1 - remove setRedeemable() (it is not mentioned in the governance docs anyway) 2 - set the redeemable token address in initialize()

-76: function initialize(address _weth) public initializer {
+76: function initialize(address _weth, address _redeemable) public initializer {
77:         __UUPSUpgradeable_init();
78:         __ReentrancyGuard_init();
79:         __Ownable_init();
80: 
81:         if (!_weth.isContract()) {
82:             revert CtrlAddressNotContract(_weth);
83:         }
84:         weth = _weth;
+           if (!_redeemable.isContract()) {
+                 revert CtrlAddressNotContract(_redeemable);
+           }
+           redeemable = IRedeemable(_redeemable);
85:     }

Duplicate of #44