sherlock-audit / 2023-12-ubiquity-judging

2 stars 2 forks source link

0xnirlin - `addAmoMinter` cannot be used in current state and will always revert #210

Closed sherlock-admin2 closed 10 months ago

sherlock-admin2 commented 10 months ago

0xnirlin

high

addAmoMinter cannot be used in current state and will always revert

Summary

addAmoMinter was taken from frax and in ubiquity context cannot be used.

Vulnerability Detail

addAmoMinter was taken from frax, but ubiquity has the different architecture of amo minter that is an EOA , and as sponsor confirmed they don't need the following line in addAmoMinter, and if deployed in its current state would need to upgrade the contracts as it will keep reverting.

   uint256 collatValE18 = IDollarAmoMinter(amoMinterAddress)
            .collateralDollarBalance();

295590442-9e4772d7-c59f-43e5-8a34-31463c3003ab

So functionality becomes useless and breaks the core functionality to be able to generate the yield.

Impact

Amo can't be added, and can't allocate assets for yeild.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L608-L621

Tool used

Cats

Recommendation

Remove the line altogether.

sherlock-admin2 commented 10 months ago

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

auditsea commented:

It's protocol decision, if required, it can be upgraded

sherlock-admin2 commented 10 months ago

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

auditsea commented:

It's protocol decision, if required, it can be upgraded

nevillehuang commented 10 months ago

Invalid, out of scope and speculating on architecture of AMOminter will be. Additionally. addAmoMinter is sufficiently tested here