New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Assets may be lost when calling unprotected AutoPxGlp::compound
function
#137
Comments
Picodes marked the issue as duplicate of #183 |
Picodes marked the issue as duplicate of #185 |
Picodes marked the issue as selected for report |
JeeberC4 marked the issue as not a duplicate |
JeeberC4 marked the issue as primary issue |
We're using the following combination of mechanics in order to make it front-running
Both will result in a higher frequency of the vault compounding its rewards and less resources available for potential attackers. |
Lines of code
https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGlp.sol#L210
https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/PirexGmx.sol#L497-L516
Vulnerability details
Impact
Compounded assets may be lost because
AutoPxGlp::compound
can be called by anyone and minimum amount of Glp and USDG are under caller's control. The only check concerning minValues is that they are not zero (1 will work, however from the perspective of real tokens e.g. 1e6, or 1e18 it's virtually zero). Additionally, internal smart contract functions use it as well with minimal possible value (e.g.beforeDeposit
function)Proof of Concept
compound
function calls PirexGmx::depositGlp, that uses external GMX reward router to mint and stake GLP.https://snowtrace.io/address/0x82147c5a7e850ea4e28155df107f2590fd4ba327#code
Next
GlpManager::addLiquidityForAccount
is calledhttps://github.com/gmx-io/gmx-contracts/blob/master/contracts/core/GlpManager.sol#L103
which in turn uses vault to swap token for specific amount of USDG before adding liquidity:
https://github.com/gmx-io/gmx-contracts/blob/master/contracts/core/GlpManager.sol#L217
The amount of USGD to mint is calcualted by GMX own price feed:
https://github.com/gmx-io/gmx-contracts/blob/master/contracts/core/Vault.sol#L765-L767
In times of market turbulence, or price oracle manipulation, all compound value may be lost
Tools Used
VS Code, arbiscan.io
Recommended Mitigation Steps
Don't depend on user passing minimum amounts of usdg and glp tokens. Use GMX oracle to get current price, and additionally check it against some other price feed (e.g. ChainLink)
The text was updated successfully, but these errors were encountered: