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
Underlying assets stealing in AutoPxGmx
and AutoPxGlp
via share price manipulation
#275
Labels
3 (High Risk)
Assets can be stolen/lost/compromised directly
bug
Something isn't working
H-05
primary issue
Highest quality submission among a set of duplicates
selected for report
This submission will be included/highlighted in the audit report
upgraded by judge
Original issue severity upgraded from QA/Gas by judge
Comments
code423n4
added
2 (Med Risk)
Assets not at direct risk, but function/availability of the protocol could be impacted or leak value
bug
Something isn't working
labels
Nov 28, 2022
Picodes marked the issue as duplicate of #407 |
Picodes marked the issue as selected for report |
c4-judge
added
the
selected for report
This submission will be included/highlighted in the audit report
label
Dec 21, 2022
Picodes changed the severity to 3 (High Risk) |
c4-judge
added
3 (High Risk)
Assets can be stolen/lost/compromised directly
upgraded by judge
Original issue severity upgraded from QA/Gas by judge
and removed
2 (Med Risk)
Assets not at direct risk, but function/availability of the protocol could be impacted or leak value
labels
Dec 21, 2022
JeeberC4 marked the issue as not a duplicate |
C4-Staff
added
primary issue
Highest quality submission among a set of duplicates
and removed
duplicate-407
labels
Jan 10, 2023
JeeberC4 marked the issue as primary issue |
This was referenced Jan 10, 2023
This was referenced Jan 10, 2023
Closed
This was referenced Jul 5, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
3 (High Risk)
Assets can be stolen/lost/compromised directly
bug
Something isn't working
H-05
primary issue
Highest quality submission among a set of duplicates
selected for report
This submission will be included/highlighted in the audit report
upgraded by judge
Original issue severity upgraded from QA/Gas by judge
Lines of code
https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/vaults/PirexERC4626.sol#L156-L165
https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/vaults/PirexERC4626.sol#L167-L176
Vulnerability details
Impact
pxGMX and pxGLP tokens can be stolen from depositors in
AutoPxGmx
andAutoPxGlp
vaults by manipulating the price of a share.Proof of Concept
ERC4626 vaults are subject to a share price manipulation attack that allows an attacker to steal underlying tokens from other depositors (this is a known issue of Solmate's ERC4626 implementation). Consider this scenario (this is applicable to
AutoPxGmx
andAutoPxGlp
vaults):AutoPxGmx
vault;deposit
function (PirexERC4626.sol#L60), the amount of shares is calculated using thepreviewDeposit
function:convertToShares
function:19e18 * 1 / 10e18 == 1
;Tools Used
Manual review
Recommended Mitigation Steps
Consider either of these options:
deposit
function ofPirexERC4626
, consider requiring a reasonably high minimal amount of assets during first deposit. The amount needs to be high enough to mint many shares to reduce the rounding error and low enough to be affordable to users.The text was updated successfully, but these errors were encountered: