search

sherlock-audit / 2023-01-uxd-judging

3 stars 1 forks source link

R2 - Rebalancing issues #420

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

R2

high

Rebalancing issues

Summary

Rebalancing is a crucial part of the protocol. But there are some issues with it:

Vulnerability Detail

  1. Rebalancing can't be performed if PerpDepository.spotSwapper not set
  2. You have rebalancing only for negative PnL

Impact

Unbalanced circulation of UXD may lead to overinflation or opposite result. Both ways are bad for the protocol.

Code Snippet

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/integrations/perp/PerpDepository.sol#L158

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/integrations/perp/PerpDepository.sol#L507

Tool used

Manual Review

Recommendation

  1. Because spotSwapper is a critical variable, save it in the contract constructor
  2. Add functions to do rebalancing in case of positive PnL (uncomment)
WarTech9 commented 1 year ago
  1. spotSwapper is set as part of contract setup during deployment process here: https://github.com/sherlock-audit/2023-01-uxd/blob/334b38b582d794c76a2062e672804d99fc24675c/scripts/optimism/2_deploy_perp_depository.ts#L85

This is the script used to deploy PerpDepository, thus, there is no chance this is not initialized.

  1. Rebalancing positive PnL can not be safely performed without impacting the protocol USDC deposits (insurance fund). So this is disabled on purpose.
hrishibhat commented 1 year ago

Considering this issue as informational based on Sponsor comments