code-423n4 / 2023-07-tapioca-findings

15 stars 10 forks source link

TricryptoNativeStrategy.setTricryptoLPGetter() fails to remove liquidity from the old lpGetter when setting a new lpGetter, as a result, funds will be lost. #59

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/curve/TricryptoNativeStrategy.sol#L141-L146

Vulnerability details

Impact

Detailed description of the impact of this finding. TricryptoNativeStrategy.setTricryptoLPGetter() fails to remove liquidity from the old lpGetter when setting a new lpGetter, as a result, funds will be lost to the old lpGetter. Similar problem occurs at ConvexTricryptoStrategy.setTricryptoLPGetter().

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

TricryptoNativeStrategy.setTricryptoLPGetter() can be used by the owner to set a new lpGetter.

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/curve/TricryptoNativeStrategy.sol#L141-L146

However, it is important to remove liquidity from the old lpGetter and add it to the new lpGetter. Unfortunately, the TricryptoNativeStrategy.setTricryptoLPGetter() function fails to do so. As a result, funds will be lost to the old lpGetter.

Tools Used

VSCode

Recommended Mitigation Steps

remove liquidity from the old lpGetter when setting the new lpGetter.

Assessed type

Token-Transfer

c4-pre-sort commented 1 year ago

minhquanym marked the issue as primary issue

0xRektora commented 1 year ago

Duplicate of #52

c4-sponsor commented 1 year ago

cryptotechmaker marked the issue as disagree with severity

cryptotechmaker commented 1 year ago

Should be low or maximum medium severity. Replacing the property is unlikely and strategies are also replaceable. It's a duplicate as well

c4-sponsor commented 1 year ago

cryptotechmaker (sponsor) confirmed

c4-judge commented 1 year ago

dmvt marked the issue as duplicate of #52

c4-judge commented 1 year ago

dmvt changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

dmvt marked the issue as grade-b

c4-judge commented 1 year ago

dmvt marked the issue as grade-a