sherlock-audit / 2023-03-taurus-judging

4 stars 0 forks source link

0x52 - GmxYieldAdapter#collectYield continues to function even on a paused vault #182

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

0x52

medium

GmxYieldAdapter#collectYield continues to function even on a paused vault

Summary

In the readme it states: "Users can exit paused vaults, but otherwise no significant action should be possible on them." However, it is still possible to collect yield from GMX when the vault is paused. This is because GmxYieldAdapter#collectYield lacks the whenNotPaused modifier.

Vulnerability Detail

See summary

Impact

Yield can still be collected even when the vault is paused, which is problematic depending on why the vault is paused

Code Snippet

GmxYieldAdapter.sol#L32-L35

Tool used

Manual Review

Recommendation

Add the whenNotPaused modifer to GmxYieldAdapter#collectYield

Sierraescape commented 1 year ago

https://github.com/protokol/taurus-contracts/pull/83

spyrosonic10 commented 1 year ago

Escalate for 10 USDC

This is low/info issue. collectYield() method neither change any state in vault contract nor impact any functionality in vault. Also there is no loss of fund scenario.

collectYield() is calling an external contract aka rewardRouter of gmx. I have looked at source code of RewardRouter of gmx and the method being called is also safe in RewardRouter of gmx and both these methods are unpausable also in gmx. There is no reason to add modifier whenNotPaused.

Hence this issue does not need criteria for High and Medium, it can be considered as Low/Info.

sherlock-admin commented 1 year ago

Escalate for 10 USDC

This is low/info issue. collectYield() method neither change any state in vault contract nor impact any functionality in vault. Also there is no loss of fund scenario.

collectYield() is calling an external contract aka rewardRouter of gmx. I have looked at source code of RewardRouter of gmx and the method being called is also safe in RewardRouter of gmx and both these methods are unpausable also in gmx. There is no reason to add modifier whenNotPaused.

Hence this issue does not need criteria for High and Medium, it can be considered as Low/Info.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

hrishibhat commented 1 year ago

Escalation accepted

The impact of the issue is not a valid high/medium as there is no loss of funds or any other risk in this case.

sherlock-admin commented 1 year ago

Escalation accepted

The impact of the issue is not a valid high/medium as there is no loss of funds or any other risk in this case.

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.