Owner can rug GMX depositors in `PirexGmx.sol`

[code423n4]

Lines of code

https://github.com/code-423n4/2022-11-redactedcartel/blob/9e9bb60f117334da7c5d851646a168ca271575fc/src/PirexGmx.sol#L691

Vulnerability details

Proof of Concept

In the two redeem methods in PirexGmx we have the whenNotPaused modifier. Pausability should be used for inbound methods (like the deposit methods) but not for outbound (the redeem methods). This is so because in the current situation we have the possibility of a malicious or compromised owner setting paused = true; and then just renouncing ownership, which will result in all depositors being unable to redeem their deposited GMX.

Impact

This is a centralisation vulnerability that can result in 100% deposited funds loss for users. It requires a malicious/compromised owner, so it is Medium severity

Recommendation

Remove the whenNotPaused modifier from the redeem methods, leave it on the deposit methods only

[c4-judge]

Picodes marked the issue as primary issue

[c4-sponsor]

drahrealm marked the issue as sponsor disputed

[drahrealm]

Centralization issue is out of the scope. Please refer to: https://github.com/code-423n4/2022-11-redactedcartel#out-of-scope

[Picodes]

Downgrading to QA per https://github.com/code-423n4/2022-11-redactedcartel#out-of-scope

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[c4-judge]

Picodes marked the issue as grade-c