sherlock-audit / 2024-02-tapioca-judging

2 stars 2 forks source link

GiuseppeDeLaZara - `LeverageExecutor` is not working inside `BBLeverage` and `SGLeverage` #78

Closed sherlock-admin3 closed 5 months ago

sherlock-admin3 commented 5 months ago

GiuseppeDeLaZara

high

LeverageExecutor is not working inside BBLeverage and SGLeverage

Summary

The buyCollateral and sellCollateral functions inside the BBLeverage and SGLeverage contracts are non-functional as there is no implementation of getCollateral and getAsset in the current LeverageExecutor interface.

Vulnerability Detail

BBLeverage::buyCollateral and SGLeverage::buyCollateral are calling LeverageExecutor::getCollateral.

BBLeverage::sellCollateral and SGLeverage::buyCollateral are calling LeverageExecutor::getAsset.

However, these functions have completely different interfaces inside all the implementations of BaseLeverageExecutor, e.g. AssetToSGLPLeverageExecutor, AssetTotsDaiLeverageExecutor, SimpleLeverageExecutor.

It seems in the previous versions of Tapioca leverageExecutor had the interface that is now expected by BBLeverage and SGLeverage. This was probably introduced during the migration to new leverageExecutor.

Neverthless, the current buyCollateral and sellCollateral functions are non-functional as there is no implementation of getCollateral and getAsset in the current LeverageExecutor interface.

Impact

The buyCollateral and sellCollateral function inside the BBLeverage and SGLeverage contracts are non-functional as there is no implementation of getCollateral and getAsset in the current LeverageExecutor interface. As this is a core functionality of the leverage contracts, it is a high-severity issue.

Code Snippet

Tool used

Manual Review

Recommendation

Change the logic inside the BBLeverage and SGLeverage contracts to use the correct interface of LeverageExecutor for getAsset and getCollateral functions, and pass the correct arguments to these functions.

Duplicate of #115

sherlock-admin3 commented 5 months ago

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

WangAudit commented:

BaseLeverageExecutor doesn't inherit ILeverageExecutor. If we look at Market.sol (this is where we declare leverageExecutor) it's ILeverageExecutor type. And this interface indeed has assetId as an input parameter -> BaseLeverageExecutor will not be called here cause BaseLeverageExecutor doesn't inherit ILeverageExecutor