ConvexTricryptoStrategy will stop working after a new ``lpGetter`` is set by function setTricryptoLPGetter().

[code423n4]

Lines of code

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/convex/ConvexTricryptoStrategy.sol#L179-L184

Vulnerability details

Impact

Detailed description of the impact of this finding. ConvexTricryptoStrategy will stop working after a new lpGetter is set by function setTricryptoLPGetter(). The main reason is that setTricryptoLPGetter() does not clear the allowance for the old lpGetter and does not set type(uint256).max for the lpToken. Similar problem occurs for TricryptoNativeStrategy.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.

ConvexTricryptoStrategy.setTricryptoLPGetter() allows the owner to set a new lpGetter:

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/convex/ConvexTricryptoStrategy.sol#L179-L184

However, although it clears allowance for the old lpGetter and set a max allowance for the new lpGetter for wrappedNative tokens. Such clearance and reset is not done for the lptoken. As a result, the new lpGetter will not work for lptoken. The ConvexTricryptoStrategy contract will thus not be functioning.

Tools Used

VCcode

Recommended Mitigation Steps

We need to take care of the lptoken allowance as well:

 function setTricryptoLPGetter(address _lpGetter) external onlyOwner {
        emit LPGetterSet(address(lpGetter), _lpGetter);
        wrappedNative.approve(address(lpGetter), 0);
+       lptoken.approve(address(lpGetter), 0);
        lpGetter = ITricryptoLPGetter(_lpGetter);
        wrappedNative.approve(_lpGetter, type(uint256).max);
+       lptoken.approve(_lpGetter, type(uint256).max);
    }

Assessed type

ERC20

[c4-pre-sort]

minhquanym marked the issue as primary issue

[c4-sponsor]

0xRektora marked the issue as disagree with severity

[0xRektora]

Should be low severity. Strategies are easily replaceable, and scenario might never occur.

[c4-sponsor]

0xRektora marked the issue as sponsor confirmed

[dmvt]

I agree with the sponsor here on severity. This setting is reversible and fixable by deploying a new strategy. No funds are at risk as far as I can tell.

[c4-judge]

dmvt changed the severity to QA (Quality Assurance)

[c4-judge]

dmvt marked the issue as grade-b

[c4-judge]

dmvt marked the issue as grade-a