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...

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...

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...

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...

Possible lost msg.value in function distributeFees

Lines of code Vulnerability details Author: rotcivegaf Impact The function distributeFees don't checks if the tokenId exists and if the owner send value to the an non-exists tokenId, this one will stuck in the contract until the tokenId will minted, and take by the owner of the minted token that...

Incorrect fee distribution due to reorg

Lines of code Vulnerability details Impact In the PostTxProcessing hook, new events are processed immediately and the addresstokenid mapping is registered to the keeper. The keeper throw an error upon duplicated registration. When there is a chain reorg that changed the registration on the EVM...

Need to check the _mint() status before increment the token count .

Lines of code Vulnerability details Impact Its possible to increase the tokenIdTracker even mint function failed. The mint status must be checked before increment tokenIdTracker value . Proof of Concept function registeraddress recipient public onlyUnregistered returns uint256 tokenId address...

evm_hooks ignores some important errors

Lines of code Vulnerability details Impact Some contracts and some Turnstile tokens nfts wll not be able to receive CSR fees forever. Proof of Concept In evmhooks.go, the PostTxProcessing will call h.processEventsctx, receipt to handle Register and Assign events from Turnstile contract first:...

Incorrect revenue calculation will lead to revenue theft through proxy attacks

Lines of code Vulnerability details Impact The incorrect way revenue is calculated can lead to CSR being stolen through proxy attacks, which is likely to lead the ecology into CSR bribery war. Eventually, this feature will translate into reduced gas fees for all transactions, regardless of whethe...

The check for msg.value in distributeFees is unnecessary and may cause error

Lines of code Vulnerability details Impact Transactions may fail because of this redundant check. Proof of Concept The function distributeFees will revert if msg.value == 0: function distributeFeesuint256 tokenId public onlyOwner payable if msg.value == 0 revert NothingToDistribute; balancestoken...

WithContext is not called on state changing operation

Lines of code Vulnerability details Impact Any state transition function must update keeper using WithContext function having latest context, otherwise incorrect state will be used. It was observed that CallEVM function misses the same Proof of Concept 1. Observe the CallEVM function func k Keepe...

Nonce is not updated after EVM Transaction

Lines of code Vulnerability details Impact It was observed that nonce is not updated while performing the EVM transaction. This could lead to Replay attacks Proof of Concept 1. Observe the CallEVM function func k Keeper CallEVM ctx sdk.Context, from common.Address, to common.Address, amount...

Funds are locked if can’t transfer reward to recipient in withdraw

Lines of code Vulnerability details Impact When recipient not able to received reward when call withdraw, as natspec: If contract is using proxy pattern, it's possible to register retroactively, however past fees will be lost. We not handle that case to get locked funds back. We should add...

ETH can get stuck (and possibly be stolen as well).

Lines of code Vulnerability details Impact The Turnstile contract contains a distributeFees function which can only be called by owner to assign and distribute fee for a tokenId. However the function does not validates the tokenId input value. So it is possible for the owner to provide any uint25...

Turnstile.distributeFees doesn't check if nft exists

Lines of code Vulnerability details Impact Turnstile.distributeFees doesn't check if nft exists and allows to distribute fees to token that doesn't exist. Proof of Concept function distributeFeesuint256 tokenId public onlyOwner payable if msg.value == 0 revert NothingToDistribute; balancestokenId...

User can redirect fees by using a proxy contract

Lines of code Vulnerability details Impact For any given tx, the fees are sent to its recipient To. Anybody can register an address using the Turnstile contract. Thus, a user is able to create a proxy contract with which they execute other smart contracts. That way, the fees are sent to their own...

Any address can assign itself with any existing NFT.

Lines of code Vulnerability details Impact The assign function of Turnstile contract does not validate whether the caller should be allowed to invoke this function. Any address can invoke this function with any pre-existing tokenId value as the input and the function will get executed successfull...

M-01 Usage of deprecated sendValue to send eth

Lines of code Vulnerability details Impact The recommended way to send ether is with call function. Using transfer or sendValue could lead to running out of gas, due to the fact that it is predefined and the transfer will fail, in such scenario there won't be way to withraw the amount from the...

Upgraded Q -> M from #418 [1669043788344]

Judge has assessed an item in Issue 418 as M risk. The relevant finding follows: 1. LBRouter's swapAVAXForExactTokens not working as intended LBRouter's swapAVAXForExactTokens will only work when sending exact msg.value = amountIn0. The functionality which returns excess funds to the user in the...

Upgraded Q -> M from #307 [1669043813221]

Judge has assessed an item in Issue 307 as M risk. The relevant finding follows: L-1. Wrong comparison sign Description The function swapAVAXForExactTokens will revert when msg.value amountsIn0 because amountsIn0 - msg.value will always cause underflow. Solidity version ^0.8.0 is used, so it will...

Upgraded Q -> M from 300 [1668888076013]

Judge has assessed an item in Issue 300 as Medium risk. The relevant finding follows: L-03 Full minimal bond amount is slashed, instead of a percentage - Duplicate of 307 --- The text was updated successfully, but these errors were encountered: All reactions...

GiantMevAndFeesPool::afterTokenTransfer doesn't update claimed amount of sender

Lines of code Vulnerability details Impact After a token transfer of GiantMevAndFeesPool's GiantLP, the receiver gets their claimed amount updated to the correct value, but the sender does not. If more than zero tokens were transferred, that amount in the sender's future rewards will be lost, and...

Users of StakingFundsVault can never redeem their LP token once staking has started

Lines of code Vulnerability details The burnLPToken of a Fees & MEV vault allow users to burn LP tokens in exchange of ETH. Quoting the documentation Every user has a right to leave the LSD network at anytime. A depositor/staker can simply sell their LP tokens to someone else or burn to redeem ET...

Medium: Users receive an incorrect account of their accumulated ETH in GiantMevAndFeesPool.

Lines of code Vulnerability details Description GiantMEVAndFeesPool exposes previewAccumulatedETH for users to view how much ETH they have accumulated through the vault: function previewAccumulatedETH address user, address calldata stakingFundsVaults, LPToken calldata lpTokens external view retur...

Single-step process for critical ownership transfer/renounce is risky

Lines of code Vulnerability details Single-step process for critical ownership transfer/renounce is risky Impact The following contracts and functions, allow owners to interact with core functions such as: execute, rawExecute and setApproval in OwnableSmartWallet registerKnotsToSyndicate,...

Medium: DAO can drain all funds of all node runners immediately

Lines of code Vulnerability details Description The DAO can steal all funds of all node runners in the system, which means the system is heavily centralized. function executeAsSmartWallet address nodeRunner, address to, bytes calldata data, uint256 value external payable onlyDAO address smartWall...

Users can block other users from redeeming their ETH in Vaults

Lines of code Vulnerability details The burnLPToken of a protected vault allow users to burn LP tokens in exchange of ETH or dETH. In the case of ETH, ie when the BLS key has not had its derivatives minted yet, the function checks the liquidity is not fresh by checking...

batchRotateLPTokens in GiantSavETHVaultPool can be used to steal LPTokens

Lines of code Vulnerability details Impact real LPTokens can be transferred out of GiantSavETHVaultPool through fake stakingFundsVaults provided by an attacker. Proof of Concept batchRotateLPTokens takes in stakingFundsVaults, oldLPTokens, newLPTokens and rotate amounts from old to new tokens. Th...

Use of transfer() instead of call() to send eth

Lines of code Vulnerability details Use of transfer instead of call to send eth Impact Use of transfer might render ETH impossible to withdraw because after istanbul hardfork, there is increases in the gas cost of the SLOAD operation and therefore breaks some existing smart contracts. Those...

Slashing can prevent Protected Staking Pool depositors from redeeming their ETH

Lines of code Vulnerability details The documentation states that: The Protected Staking Pool is free from any slashing and leaking risk. While the penalties affect the node operator first, then the Fees and MEV Pool stakers, if the slashing amount is higher, it will also impact the Protected...

EOAs and system contracts can be blocked from some actions by continuously transferring them zero LP tokens

Lines of code Vulnerability details Impact LPToken contains the map lastInteractedTimestamp which maps addresses to timestamps and is updated for from and to addresses after a token transfer. Many operations will check the last interaction time of an address and revert if it's too recent to comba...

HIGH : The Giant pools can be drained by any user.

Lines of code Vulnerability details Description In GiantSavETHVaultPool.sol, batchDepositETHForStaking is used to deposit held funds to savETHPool vault: function batchDepositETHForStaking address calldata savETHVaults, uint256 calldata ETHTransactionAmounts, bytes calldata blsPublicKeys, uint256...

Medium: Giant pools are prone to user griefing, preventing their holdings from being staked.

Lines of code Vulnerability details Description batchRotateLPTokens in GiantMevAndFeesPool allows any user to rotate LP tokens of stakingFundsVaults around. function batchRotateLPTokens address calldata stakingFundsVaults, LPToken calldata oldLPTokens, LPToken calldata newLPTokens, uint256 callda...

Med: withdrawDETH is not functional for array lengths greater than one.

Lines of code Vulnerability details Description The withdrawDETH function is used in GiantSavETHVaultPool to burn user's LP tokens and grant them dETH. It loops over all input vaults and all input LPTokens, and for each one calls lpTokenETH.burnmsg.sender, amount; Before that, it uses...

Any user can block other users from interacting with LPTokens by transferring tiny amounts and resetting their lastInteractedTimestamp

Lines of code Vulnerability details Impact LPTokens in several different contracts have an afterTokenTransfer hook, which updates the last interacted timestamp. Because users can send LPTokens to any other user, this resets their last interacted timestamp which is used for several key actions whi...

Medium: Vaults can be griefed to not be able to be used for deposits

Lines of code Vulnerability details Description Interaction with SavETHVault and StakingFundsVault require a minimum amount of MINSTAKINGAMOUNT. In order to be used for staking, there needs to be 24 ETH or 4 ETH for the desired BLS public key in those vaults. The issue is that vaults can be grief...

Unchecked transfer

Lines of code Vulnerability details Impact The return value of an external transfer/transferFrom call is not checked Proof of Concept you can check Vulnerability in code here: Tools Used Recommended Mitigation Steps Use SafeERC20, or ensure that the transfer/transferFrom return value is checked...

value can be burned in burnLPToken but nothing withdrawn

Lines of code Vulnerability details value can be burned in burnLPToken but nothing withdrawn Summary If initial amount is not big enough, value can get truncated by division of 24 ether and therefore be 0 the redemptionValue. Therefore, this value to be withdrawn would be lost. PoC // redeem savE...

batchDepositETHForStaking in GiantMevAndFeesPool.sol can be ticked to steal all ETH in the pool

Lines of code Vulnerability details Impact All Eth can be drained by fake vault addresses. Proof of Concept In batchDepositETHForStaking, stakingFundsVault is checked for its validity through StakingFundsVault sfv = StakingFundsVaultpayablestakingFundsVaulti; require...

ERC20 return values not checked

Lines of code Vulnerability details Impact 'token' is a ERC20 from LPToken.sol The ERC20.transfer and ERC20.transferFrom functions return a boolean value indicating success. This parameter needs to be checked for success. Some tokens do not revert if the transfer failed but return false instead...

bringUnusedETHBackIntoGiantPool in GiantMevAndFeesPool can be used to steal LPTokens

Lines of code Vulnerability details Impact real LPTokens can be transferred out of GiantMevAndFeesPool through fake stakingFundsVaults provided by an attacker. Proof of Concept bringUnusedETHBackIntoGiantPool takes in stakingFundsVaults, oldLPTokens, newLPTokens and rotate amounts from old to new...

HIGH: ETH is forever locked in GiantMevAndFeesPool due to unaccepted burn()

Lines of code Vulnerability details Description GiantMevAndFeesPools and StakingFundsVault implement beforeTokenTransfer transfer hooks. StakingFundsVault.sol: function beforeTokenTransferaddress from, address to, uint256 external override address syndicate = liquidStakingNetworkManager.syndicate...

QA Report

See the markdown file with the details of this report here. --- The text was updated successfully, but these errors were encountered: All reactions...

Staking Funds vault's LP holder cannot claim EIP1559 rewards after derivatives are minted for a new BLS public key that is not the first BLS public key registered for syndicate

Lines of code Vulnerability details Impact After the derivatives are minted for the first BLS public key registered for the syndicate, the Staking Funds vault's LP holder can claim the corresponding EIP1559 rewards received by the syndicate. However, after the derivatives are minted for a new BLS...

an early staker in StakingFundsVault can take other stakers eth

Lines of code Vulnerability details Impact The first stakers in a StakingFundsVault can intentionally or unintentionally take ethfrom other stakers. If they claim rewards while eth is in the contract but yet not used to stake towards a validator that eth can be counted as rewards. Proof of Concep...

Node operators cannot withdraw their ETH when they rage quit

Lines of code Vulnerability details Node operators can opt for Rage Quit, after the BLS public key is staked. In the current configuration, they will not be able to retrieve their staked ETH: withdrawETHForKnot will revert here if the lifecycle status is not INITIALSREGISTERED. The lifecycle stat...

potential reentrancy risk in _distributeETHRewardsToUserForToken

Lines of code Vulnerability details potential reentrancy risk in distributeETHRewardsToUserForToken Impact Reentrancy risk can potentially be demaging Proof of Concept distributeETHRewardsToUserForToken, which in turn calls bool success, = recipient.callvalue: due"";. As recipient of ETH, the...

HIGH: Vault or pool funds can be stolen by any user who holds LP tokens.

Lines of code Vulnerability details Description The distributeETHRewardsToUserForToken is implemented in SyndicateRewardsProcessor and used in derived contracts: GiantMevAndFeePool and StakingFundsVault. It sends out rewards to the given user according to the current accumulatedETHPerLPShare and...

HIGH : The Giant pools can be drained by any user.

Lines of code Vulnerability details Description In GiantSavETHVaultPool.sol, batchDepositETHForStaking is used to deposit held funds to savETHPool vault: function batchDepositETHForStaking address calldata savETHVaults, uint256 calldata ETHTransactionAmounts, bytes calldata blsPublicKeys, uint256...

Unbounded loop can run out of gas in withdraw and unstake operations

Lines of code Vulnerability details Unbounded loop can run out of gas in withdraw and unstake operations Impact There are no bounds on the number of orders in the loop, this can run out of gas due to cost of the operations that can vary by the time. Proof Of Concept function withdrawLPTokensLPTok...