GatewayGuardians - UbiquityPool: Full collateral withdrawal during redemption delay can leave users without the collateral or uAD

[sherlock-admin2]

GatewayGuardians

medium

UbiquityPool: Full collateral withdrawal during redemption delay can leave users without the collateral or uAD

Summary

• When the collateralPool reaches its limit, the UbiquityPool is paused, and no new mints and redeems are allowed.
• Using amoMinterBorrow, the "AMO" contract can potentially withdraw all deposited collateral and use it to earn yield.
• If there are users who have completed part 1 of redemption (i.e., burning uAD) and are waiting for the redemption delay to pass to withdraw their collateral, the borrow function can withdraw all funds, leaving the users without the uAD or their collateral.

Vulnerability Detail

This issue is also dependent upon how the AMO system works. At this point, the devs have not decided on the final implementation.

• When a user, like Alice, initiates a redemption in block 100, completes part 1, and is awaiting the redemption delay.
• If another user, Bob, mints new Dollar Tokens in the same block after Alice, causing the collateral pool to reach its limit.
• The pool is paused in block 101. At this point, the AMO minter withdraws all collateral balance to earn yield.
• In this scenario, Alice cannot collect her collateral and is stuck in limbo until the pool is resumed, and the collateral is redeposited.

Impact

• Risk to users experiencing delays in retrieving their funds.

Code Snippet

• Link to affected function: https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L574
• Following is the PoC to verify this issue.

• Add this in ubiquity-dollar/packages/contracts/test/diamond/facets/UbiquityPoolFacet.t.sol

  function test_POC() public {
      console.log("[+] Setting up users");
      address bob = makeAddr("bob");
      address groupOfUsers = makeAddr("groupOfUsers");
  
      collateralToken.mint(groupOfUsers, 50000e18);
      collateralToken.mint(bob, 50000e18);
  
      vm.prank(groupOfUsers);
      collateralToken.approve(address(ubiquityPoolFacet), 50000e18);
      vm.prank(bob);
      collateralToken.approve(address(ubiquityPoolFacet), 50000e18);
  
      vm.prank(admin);
      ubiquityPoolFacet.addAmoMinter(address(dollarAmoMinter));
  
      vm.prank(admin);
      ubiquityPoolFacet.setPriceThresholds(1000000, 1000000);
  
      console.log("============ Block 1 ============");
      console.log("[+] Users mint Dollar Tokens");
      vm.prank(groupOfUsers);
      ubiquityPoolFacet.mintDollar(0, 40000e18, 39500e18, 40000e18);
  
      console.log("[+] Some users redeems Dollar Tokens");
      vm.prank(groupOfUsers);
      ubiquityPoolFacet.redeemDollar(0, 20000e18, 18000e18);
  
      console.log("[+] Bob mints uAD and collateral balance reaches its pool ceiling");
      vm.prank(bob);
      ubiquityPoolFacet.mintDollar(0, 10000e18, 9500e18, 10000e18);
  
      vm.roll(1);
  
      console.log("============ Block 2 ============");
      console.log("[+] AMO borrows all funds");
      vm.prank(address(dollarAmoMinter));
      ubiquityPoolFacet.amoMinterBorrow(50000e18);
  
      vm.roll(3);
  
      console.log("============ Block 5 ============");
      console.log("[+] Users cannot claim their collateral");
      vm.prank(groupOfUsers);
      vm.expectRevert();
      ubiquityPoolFacet.collectRedemption(0);
  }

• Output:

  Logs:
  [+] Setting up users
  ============ Block 1 ============
  [+] Users mint Dollar Tokens
  [+] Some users redeems Dollar Tokens
  [+] Bob mints uAD and collateral balance reaches its pool ceiling
  ============ Block 2 ============
  [+] AMO borrows all funds
  ============ Block 5 ============
  [+] Users cannot claim their collateral

  Tool used

Manual Review

Recommendation

• A simple solution would be to allow AMO to borrow only up to the freeCollateralBalance

  
  function amoMinterBorrow(uint256 collateralAmount) internal onlyAmoMinter {
      UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();
  
      // checks the collateral index of the minter as an additional safety check
      uint256 minterCollateralIndex = IDollarAmoMinter(msg.sender)
          .collateralIndex();

• require(collateralAmount <= freeCollateralBalance(minterCollateralIndex));

  // checks to see if borrowing is paused
  require(
      poolStorage.isBorrowPaused[minterCollateralIndex] == false,
      "Borrowing is paused"
  );
  
  // ensure collateral is enabled
  require(
      poolStorage.isCollateralEnabled[
          poolStorage.collateralAddresses[minterCollateralIndex]
      ],
      "Collateral disabled"
  );
  
  // transfer
  IERC20(poolStorage.collateralAddresses[minterCollateralIndex])
      .safeTransfer(msg.sender, collateralAmount);

  }

Duplicate of #1