Uint underflow issue

Lines of code Vulnerability details Impact Potential underflow if shares is greater than totalSupply Proof of Concept Tools Used Manual review Recommended Mitigation Steps Before the following line, check if totalSupply is greater or equal to shares --- The text was updated successfully, but thes...

incorrect commend and/or rounding issues in AutoPxGmx and AutoPxGlp

Lines of code Vulnerability details Impact The comments on the affected lines state previewWithdraw will round up. However, the implementation, an inner call to convertToShares is made, which actually calls mulDivDown. From further inspection, this pair of functions withdraw, previewWithdraw as...

First depositor can inflate share price

Lines of code Vulnerability details Impact A well-known vulnerability for ERC4626 vaults is the inflation of the share price on the first deposit. Because AutoPxGlp and AutoPxGmx use the balance of the underlying asset for totalAssets and do not have an initial minimum deposit amount, they are al...

user fund lose in deposit() of PirexERC4626 contract because A MALICIOUS EARLY USER/ATTACKER CAN MANIPULATE THE VAULT’S PRICEPERSHARE TO TAKE AN UNFAIR SHARE OF FUTURE USERS’ DEPOSITS

Lines of code Vulnerability details Impact This is a well-known attack vector for new contracts that utilize pricePerShare for accounting. Attacker can cause totalAssets / totalSupply ratio to go as high as he wants and then because of rounding error in convertToShares lower amount of share would...

Possible double spending issue for PirexERC4626 vault

Lines of code Vulnerability details Impact Solmate's ERC20 does not provide option to increase/decrease allowance, and only option to do so is by setting it via approve - which sets this amount directly. This poses a problem of double spending, when a user want to check current allowance, and bad...

The recipient address check during handling register events will confuse users and block the users contracts registering

Lines of code Vulnerability details Impact The check about if the receiver account exists in the evm store doesn't make sense and will cause users to encounter a confusing exception. And the RegisterEvent function will not throw an exception to revert the tx, the source contract will be wrote in...

Attackers can manipulate ERC4626 price per share to take an unfair share of future users

Lines of code Vulnerability details Impact The attacker can get funds from future users, and the future users will lose their funds. Proof of Concept A malicious early user can deposit with 1 wei of asset token and get 1 wei of shares. Then he/she can send 10000e18 - 1 of asset tokens and inflate...

Potential uint underflow

Lines of code Vulnerability details Impact Potential underflow if shares is greater than totalSupply Proof of Concept Tools Used Manual review Recommended Mitigation Steps Before the following line, check if totalSupply is greater or equal to shares --- The text was updated successfully, but thes...

NameWrapper: expired names behave unwrapped

Lines of code Vulnerability details Impact expired Names are supposed to be unregistered, but it behaves like unwrapped parent with CANNOTCREATESUBDOMAIN fuse can "create" again an expired name parent can ENS.setSubdomainOwner before burning CANNOTCREATESUBDOMAIN to be able to use the subdomain...

Uniswap V3 swaps are vulnerable to a sandwich attack

Lines of code Vulnerability details Impact As the amountOutMinimum is always hardcoded with 1 in the AutoPxGmx, then the Uniswap V3 swap is vulnerable to a frontrun / sandwich attack. Proof of Concept Even though the amountOutMinimum may never be 0 because it is prevented against it: if...

fee loss in AutoPxGmx and AutoPxGlp and reward loss in AutoPxGlp by calling PirexRewards.claim(pxGmx/pxGpl, AutoPx*) directly which transfers rewards to AutoPx* pool without compound logic get executed and fee calculation logic and pxGmx wouldn't be executed for those rewards

Lines of code Vulnerability details Impact Function compound in AutoPxGmx and AutoPxGlp contracts is for compounding pxGLP and additionally pxGMX rewards. it works by calling PirexGmx.claimpx, this to collect the rewards of the vault and then swap the received amount to calculate the reward,...

fee distribution is only for the msg.to contract, instead of the to address of the call traces, which is not reasonable for the gas economic system.

Lines of code Vulnerability details Impact The target contract of the fee distribution is got by contract := msg.To in the evmhooks.go . So the fee distribution is only for the msg.to contract, instead of the to address of the call traces. It means that any one use a contract wallet or setup a...

PirexERC4626 and PirexGmx doesn't support erc20 tokens with fee on transfer

Lines of code Vulnerability details PirexERC4626 and PirexGmx doesn't support erc20 tokens with fee on transfer Vulnerability details There are ERC20 tokens that charge fee for every transfer / transferFrom. Different functions assumes assumes that the received amount is the same as the transfer...

Potential PirexReward's producerTokens's rewardToken unsynced with PirexGmx rewardToken can miss calculate the actual reward for user

Lines of code Vulnerability details Impact Potential PirexReward's producerTokens's rewardToken unsynced with PirexGmx rewardToken can miss calculate the actual reward for user Proof of Concept PirexReward initialization does not include rewardToken initialization for producerTokens. Meanwhile...

First depositer can break Vault share distributions

Lines of code Vulnerability details The calculation of exchange rate for shares in PirexERC4626 Vault is done by dividing the total supply of shares by the totalAssets of the vault. The first depositor can mint a very small number of shares, then donate to the vault to manipulate the share price...

A registered contract won't earn fees if _recipient is a fresh address

Lines of code Vulnerability details Impact Users might fall victims of a false positive: if they use a fresh account as an NFT recipient during contract registration, the transaction won't revert, but the registered contract will never earn fees for the token holder. And since a contract can be...

Underlying assets stealing in AutoPxGmx and AutoPxGlp via share price manipulation

Lines of code Vulnerability details Impact pxGMX and pxGLP tokens can be stolen from depositors in AutoPxGmx and AutoPxGlp 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 toke...

Potential mismanaging of the access and roles

Lines of code Vulnerability details Impact In initiateMigration method we are setting migratedTo to an address of the potential migration contract Later on, if completeMigration is called, it basically checks if the provided oldContract is equal to migratedTo when we enter the migrateReward metho...

The _recipient address has no limits to the amount of tokenID (NFTs) it can own

Lines of code Vulnerability details Impact Since the recipient address has no limit to the number of tokenIds it can hold, this makes it possible for an attacker to call the register function many times with different addresses and send many tokenIds to the same recipient, which could cause a...

AutoPxGlp and AutoPxGmx are not compliant with ERC4626 standard

Lines of code Vulnerability details Proof of Concept In both contracts, the previewWithdraw function has the same body // Calculate shares based on the specified assets' proportion of the pool uint256 shares = convertToSharesassets; // Save 1 SLOAD uint256 totalSupply = totalSupply; // Factor in...

There is no re-register o re-assign function

Lines of code Vulnerability details Impact There is no re-register or re-assign option for the smart contracts. Let's assume a smart contract is registered either through the register function with a new NFT minted or the assign function to an existing NFT. However, if somehow, the NFT is burned ...

NameWrapper: Cannot prevent transfer while upgrade even with CANNOT_TRANSFER fuse regardless of the upgraded NameWrapper's implementation

Lines of code Vulnerability details Impact Upon upgrade to a new NameWrapper contract, owner of the node will be set to the given wrappedOwner. Since the node will be burned before calling the upgraded NameWrapper, the upgraded NameWrapper cannot check the old owner. Therefore, no matter the...

A malicious user might call AutoPxGmx.compound() with a higher fee than poolFee.

Lines of code Vulnerability details Impact A malicious user might call AutoPxGmx.compound with a higher fee than poolFee. As a result, there would be a fund loss for the users because they paid more than expected for the swap router. Proof of Concept As we can see here, Uniswap V3 introduces...

Reentrancy potential

Lines of code Vulnerability details Impact A reentrancy attack could be performed and can result in exploit and losing funds Proof of Concept Tools Used Manual review Recommended Mitigation Steps Use nonReentrant modifier imported from solmate/utils/ReentrancyGuard.sol --- The text was updated...

Lack Of Proper Access Control Might Lead To User Getting Lesser Rewards

Lines of code Vulnerability details Impact We can call the function userAccrue for some other user and make their rewards lesser then they expect. In the function it calculates the rewards for a user that are being accrued over a period of time. The math to calculate how much reward a user has...

Functions like AutoPxGmx.withdraw and AutoPxGmx.redeem do not provide effective slippage control

Lines of code Vulnerability details Impact As shown below, calling the AutoPxGmx.withdraw and AutoPxGmx.redeem functions would execute compoundpoolFee, 1, 0, true, which uses the hardcoded 1 as the amountOutMinimum input of the AutoPxGmx.compound function to further call the...

A malicious early user/attacker can manipulate the pxGmx's pricePerShare to take an unfair share of future user's deposits

Lines of code Vulnerability details Impact An attacker/early user can deposit 1 wei in the vault and increase the price per share by sending a very high value of the underlying directly to the vault, causing next vault depositors to: not be able to deposit less than the very high share price set ...

Lack of input validation to check whether the tokenId of the NFT exists or not - this lead to misallocation of fee earned

Lines of code Vulnerability details Impact In the distributeFees function, there is no input validation to check whether the tokenId of the NFT exists or not. If a caller inputs tokenId that does not exist, the fee earned will be added to the balance of tokenId that does not exist. Although this...

Allowance underflow

Lines of code Vulnerability details Impact I think the following check is unnecessary but furthermore, there should be a comparison if the allowance amount allowed is equal or greater than shares amount. Otherwise, we would end up with underflow of the uint value Proof of Concept Tools Used Manua...

Wrong comparison

Lines of code Vulnerability details Impact I think the following check is unnecessary but furthermore, there should be a comparison if the allowance amount allowed is equal or greater than shares amount. Otherwise, we would end up with underflow of the uint value Proof of Concept Tools Used Manua...

Compounding is vulnerable to sandwich attack

Lines of code Vulnerability details Impact Function compound is called in every deposit/withdraw in AutoPxGmx contract. It claim rewards in form of gmxBaseReward and swap them to gmx token. They used amountOutMinimum from input params gmxAmountOut = SWAPROUTER.exactInputSingle...

Use Of block.timestamp Can Result In Attacker Manipulating His/Her Rewards In Their Favour

Lines of code Vulnerability details Impact If an attacker manipulates the block.timestamp in their favor then they can get higher rewards as uint256 rewards = u.rewards + u.lastBalance block.timestamp - u.lastUpdate in this equation lets say the attacker called this just now , and on the next...

Rewards calculation does not consider GMX reward rate fluctuation

Lines of code Vulnerability details Impact The current time based px rewards calculation system is not accurate, and not fair for users. Due to GMX protocol reward rate fluctuation, px users stake and claim at different time could get less or more rewards they deserve. Some users could abuse the...

DoS on claiming rewards in PirexRewards is possible

Lines of code Vulnerability details Proof of Concept The claim method in PirexRewards iterates over the rewardTokens array for a producerToken. Now this array is completely managed by the contract’s owner who can call addRewardToken which pushes a new value in that array, as many times as he...

Fuses rollover with renew

Lines of code Vulnerability details Impact User will not be able to change fuses even after actual expiry Proof of Concept 1. User A wraps his .eth domain test.eth using the wrapETH2LD function with fuse CANNOTBURNFUSES 2. This internally calls wrapETH2LD function with User A provided fuses 3. Th...

gmxBaseReward must not be the same as asset

Lines of code Vulnerability details Impact Compounding will attempt to swap/deposit all assets instead of just the rewards, which reverts because of integer overflow, which causes withdrawals to revert. Proof of Concept In AutoPxGmx.compound: PirexRewardsrewardsModule.claimasset, addressthis; //...

User can continuosly accrue rewards they are not due

Lines of code Vulnerability details Impact It is possible that block.timestamp can be manipulted by a user, thus allowing a malicious user to continuously acrue rewards they are not due, as long as the value is not 0 then rewards will be accrued function userAccrueERC20 producerToken, address use...

platform should be approved again after it's changed by the admin.

Lines of code Vulnerability details Impact In AutoPxGmx.sol and AudotPxGlp.sol, it doesn't approve properly when platform is changed. As a result, PirexGmx contract can't transfer gmx or gmxBaseReward from these contracts and the main logic won't work as expected. Proof of Concept As we can see...

previewWithdraw() in AutoPxGlp and AutoPxGmx doesn't rounds up when calculating corresponding shares for assets, even so they are overrides previewWithdraw() in PirexERC4626 which rounds up and withdraw() in all 3 contract assumes and depends on previewWithdraw() to rounds up

Lines of code Vulnerability details Impact Contracts AutoPxGlp and AutoPxGmx extends PirexERC4626 and function withdraw and previewWithdraw has been overridden in those contracts. withdraw uses function previewWithdraw to calculate number of shares need to burn for corresponding amount of assets...

Add reward token existence check in order to avoid user reward lost.

Lines of code Vulnerability details Impact The user can lost his rewards if the reward token is removed from the producerTokensproducerToken.rewardTokens list. If the reward token is removed, the rewardToken length is going to be zero, the user rewards going to be zero and the for statement will...

Price manipulation can lead to users lossing funds

Lines of code Vulnerability details H Price manipulation can lead to users lossing funds Impact Early malicious user will profit from future users' deposits while future users' will loose funds/value. Proof of Concept An early user can call the deposit function of any of the ERC4626 vaults with...

Not checking if tokenId exist on distributeFees

Lines of code Vulnerability details Impact Not checking if tokenId exist on distributeFees can set the msg.value to uncreated tokenId Proof of Concept For example an Owner mistakenly call distributeFee with a tokenId which doesn't exist, then the function will success, but unfortunately any user...

lack of unregiestered function existen

Lines of code Vulnerability details there is no function to let a smart contract to unregester from a tokenid , since a smart contract can only be regestered once its highly recommended to give it an ability to unregister from a tokenid and register again bob calls register bob has two...

Register and assign functions need more validation due to phishing vectors.

Lines of code Vulnerability details Impact The Turnstile contract is open for phishing. Since the interaction between the contracts is in atomic construction and the calls can't be controlled at low levels, it might open ways to phishing the contracts. Let's assume that there is a contract that i...

Unlimited minting of pxGmx in PirexGmx.sol which may break protocol

Lines of code Vulnerability details Impact Unlimited minting of pxGmx which may break protocol. Proof of Concept A user can call depositGmx on PirexGmx.sol and mint some pxGmx after staking some Gmx via gmxRewardRouterV2.stakeGmxamount and transferring GMX into the contract. // Transfer the...

PostTxProcessing can revert user transactions not interacting with Turnstile

Lines of code Vulnerability details Impact Any transaction, even those that don't interact with the Turnstile contract, can be reverted by the PostTxProcessing hook if there was a CSR specific error. Thus, the CSR module can impair the behavior of smart contracts not related to the module. Proof ...

AutoPxGmx.compound function can be directly called with a fee input value that is not the configured Uniswap pool fee

Lines of code Vulnerability details Impact Calling the following AutoPxGmx.withdraw and AutoPxGmx.redeem functions would execute compoundpoolFee, 1, 0, true, which uses the configured Uniswap pool fee as the fee input of the AutoPxGmx.compound function below to further call the...

tokenID that is not minted will be stored into the feeRecipient storage

Lines of code Vulnerability details Impact In the original code, tokenID is incremented just after mint function is executed. Afterwards, tokenID is stored into the feeRecipient storage. As a result, tokenID that is not minted will be stored into the feeRecipient storage. Proof of Concept In the...

NameWrapper: one can renew to DoS a Name

Lines of code Vulnerability details Impact Anyone with enough eth can make a name impossible to be used anymore Proof of Concept Below is a snippet of the proof of concept. The whole code can be found in this gist. And how to run test is in the comment in the gist. This proof of concept...

Assign Function Allows for any Other Unknown Contract Address to Assign existing token.

Lines of code Vulnerability details Impact Assign Function Allows for any Other Unknown Contract Address to Assign existing token. Proof of Concept There is no check at all to confirm that the previous token owner allows for the assignment of his tokenId. The assign function only checks the...