sherlock-audit / 2023-12-ubiquity-judging

2 stars 2 forks source link

FastTiger - In case the EOA and contract who redeemed Dollar and have not collected redemption yet makes a loss, the unclaimedPoolCollateral will be not changed. #227

Closed sherlock-admin closed 6 months ago

sherlock-admin commented 6 months ago

FastTiger

medium

In case the EOA and contract who redeemed Dollar and have not collected redemption yet makes a loss, the unclaimedPoolCollateral will be not changed.

Summary

There are risks associated with EOA and contract who redeemed Dollar and have not collected redemption such as smart contract risks. In case a loss happens, it will not be reflected in the unclaimedPoolCollateral and unclaimedPoolCollateral will be different with real unclaimed collateral in the pool.

Vulnerability Detail

The amount of unclaimed collateral in the pool is tracked by the unclaimedPoolCollateral variable. This amount is used to calculate the freeCollateralBalance().

function freeCollateralBalance( uint256 collateralIndex ) internal view returns (uint256) { UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage(); return IERC20(poolStorage.collateralAddresses[collateralIndex]) .balanceOf(address(this)) .sub(poolStorage.unclaimedPoolCollateral[collateralIndex]); }

When there is a loss to the EOA and contract, there is no way to set the unclaimedPoolCollateral variable.

This leads to an different collateralUsdBallance.

Impact

The amount of collateralUsdBallance can be lower then the real collateral USD balance.

Code Snippet

ubiquity-dollar/packages/contracts/src/dollar/LibUbiquityPool.sol::L268~L276

Tool used

Manual Review

Recommendation

Add function to allow admin to set the unclaimedPoolCollateral.

function setUnclaimedPoolCollateral(uint256 collateralIndex, uint256 amount) external only Admin { UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();

require(amount<=poolStorage.unclaimedPoolCollateral[collateralIndex], "Amount exceeds previous amount");

poolStorage.unclaimedPoolCollateral[collateralIndex]-=amount; }

sherlock-admin2 commented 6 months ago

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

auditsea commented:

It's user action, the protocol does not need to handle the case

sherlock-admin2 commented 6 months ago

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

auditsea commented:

It's user action, the protocol does not need to handle the case

nevillehuang commented 6 months ago

Invalid, unsure what loss the watson is pointing to.