Upgraded Q -> M from #251 [1670231833702]

[c4-judge]

Judge has assessed an item in Issue #251 as M risk. The relevant finding follows:

AutoPxGlp.setPlatform and AutoPxGmx.setPlatform break the vaults functionalities. Looking at AutoPxGlp.setPlatform: this admin setter allows the owner to change the pirexGmx address in AutoPxGlp. The issue is that it does not call gmxBaseReward.safeApprove(platform, type(uin256).max).

This means that users will not be able to call depositFsGlp() anymore, as it calls beforeDeposit(), which calls compound(), where a platform.depositGlp() call is made:

238: if (gmxBaseRewardAmountIn != 0) { 239: // Deposit received rewards for pxGLP 240: (, pxGlpAmountOut, ) = PirexGmx(platform).depositGlp( 241: address(gmxBaseReward), 242: gmxBaseRewardAmountIn, 243: minUsdg, 244: minGlp, 245: address(this) 246: ); 247: } This external call will revert here as the platform's gmxBaseReward allowance is 0.

Users will not only lose the ability to deposit GLP into the contracts, they also will not be able to call withdraw or redeem as both these functions call compound(): meaning they will not be able to retrieve their pxGLP.

Note that while centralization issues are mentioned in the README:

we also believe that certain privileges need to be retained by our team in order to safeguard user funds and maintain the protocol's continued operations In this case, the issue is not that this function can be used in a way to grieve users, but that it by design simply breaks the entire autoPxGlp functionality: There is no way to call gmxBaseReward.safeApprove() on a new platform.

The exact same issue happens in autoPxGmx with setPlatform, causing the compound() call to revert here.

You should add:

gmxBaseReward.safeApprove(platform, type(uin256).max) in AutoPxGlp.setPlatform. gmx.safeApprove(platform, type(uin256).max) in AutoPxGmx.setPlatform.

[c4-judge]

Picodes marked the issue as duplicate of #182

[c4-judge]

Picodes marked the issue as satisfactory