sherlock-audit / 2023-01-sentiment-judging

2 stars 0 forks source link

Bahurum - Use of [`fsGLP`] instead of [`sGLP`] in `PLVGLPController` and `RewardRouterV2Controller` #28

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

Bahurum

high

Use of [fsGLP] instead of [sGLP] in PLVGLPController and RewardRouterV2Controller

Summary

In PLVGLPController and RewardRouterV2Controllerl sGLP is used instead of fsGLP as token. This means that when depositing or redeeming fsGLP will not be added to the account collateral and the margin for the account will be underestimated, which can cause early liquidation of the account.

Vulnerability Detail

In both cases, the tokens received are ignored and not added to the account's collateral. The margin will be unexpectedly low and the account can face early liquidation.

Impact

The collateral calculated will be lower than the actual value of tokens in the account. This is unexpected by the owner of the account, and he can be liquidated earlier than expected.

Code Snippet

https://github.com/sherlock-audit/2023-01-sentiment/blob/main/controller-55/src/gmx/RewardRouterV2Controller.sol#L28-L29

https://github.com/sherlock-audit/2023-01-sentiment/blob/main/controller-55/src/plutus/PLVGLPController.sol#L28-L29

Tool used

Manual Review

Recommendation

In both contracts, use the fsGLP token at address 0x1aDDD80E6039594eE970E5872D247bf0414C8903 instead of the token sGLP at the address 0x5402B5F40310bDED796c7D0F3FF6683f5C0cFfdf.

r0ohafza commented 1 year ago

sGLP is transferrable version of fsGLP , sGLP functions as fsGLP by returning the balance of fsGLP the holder has as well as transferring fsGLP from one user to another. If fsGLP is added to the system it cannot be transferred outside of the sentiment account which would prevent liquidations. sGLP solves this issue which is why we represent the fsGLP held in the account using sGLP.

hrishibhat commented 1 year ago

Closing the issue based on sponsor's comment.