Upgraded Q -> M from #400 [1670236078897]

Judge has assessed an item in Issue 400 as M risk. The relevant finding follows: L03 - LiquidStakingManager.dao can rug node operators with executeAsSmartWallet 202: function executeAsSmartWallet 203: address nodeRunner, 204: address to, 205: bytes calldata data, 206: uint256 value 207: external...

setRelayer function not checking for null relayer address

Lines of code Vulnerability details Summary When calling the setRelayer function with a null relayer address, the function does not check for this and allows the address to be set. This issue can cause the contract to fail when calling the executeCalls function and potentially cause loss of funds...

Cross-chain request does not have expiration time

Lines of code Vulnerability details Impact Cross-chain request does not have expiration time Proof of Concept In the current implementation, a caller in source chain can perform a relayer call to starts a cross-chain request. But it is not clearly when the executor will be executing the...

Funds may be lost if processCalls is called from a contract

Lines of code Vulnerability details Impact In CrossChainRelayerArbitrum.processCalls, msg.sender is set as the address to receive any refund both excess fee refund and call value refund in the createRetryableTicket call. Arbitrium credits the refunded value to L2 balance, so if the user doesn't...

CrossChainRelayerArbitrum.processCalls() can be invoked multiple times.

Lines of code Vulnerability details Impact In the CrossChainRelayerArbitrum contract once a Call has been relayed using the relayCalls function, processCalls can be invoked multiple times by anyone for the respective Call. Due to this redundant IInbox.createRetryableTicket calls are triggered and...

Upgraded Q -> M from #400 [1670236164031]

Judge has assessed an item in Issue 400 as M risk. The relevant finding follows: L04 - LiquidStakingManager.dao can rug node operators with executeAsSmartWallet daoCommissionPercentage is used to calculate the portion of node operator network rewards that are sent to dao, when a node runner is...

Upgraded Q -> M from #38 [1670234424045]

Judge has assessed an item in Issue 38 as M risk. The relevant finding follows: L2. Pool selection for the compound is not restricted Compound function of AutoPxGmx contract is public and available for everyone. An attacker can create a custom pool with a higher fee value currently, on tests it...

ProcessedCalls event missing transaction hash

Lines of code Vulnerability details Summary The CrossChainRelayerArbitrum contract is missing the txHash parameter in the ProcessedCalls event. Impact The txHash is used to identify the transaction that was processed. Without this parameter, it will be impossible to know which transaction was...

CrossChainExecutorPolygon does not implement the executeCalls function

Lines of code Vulnerability details Impact The CrossChainExecutor contracts in the codebase are meant to follow the CrossChainExecutor interface as defined in EIP-5164. Each L2 network specific CrossChainExecutor contract is also expected to inherit and follow the ICrossChainExecutor interface...

Incorrect behavior in CrossChainExecutorPolygon contract

Lines of code Vulnerability details Description: We have discovered an issue with the CrossChainExecutorPolygon contract. When a message with a nonce that has already been executed is received, the contract does not prevent the message from being processed or display an error message. This issue...

the executor can execute fake cross-chain function call

Lines of code Vulnerability details Impact The caller on relayer side can be rugged if the executor call the executeCall on execution side directly to use consume the nonce and fake cross-chain function call. Proof of Concept The expected behavior is that: the relayer relays a cross-chain request...

Replay Attack possible by processCalls function

Lines of code Vulnerability details Impact Calls can be processed multiple times Proof of Concept The processCalls function uses the require function to check whether the nonce and calls values have been relayed, but it does not check whether the sender value is the same as the msg.sender value...

relayCalls won't work for expensive operations

Lines of code Vulnerability details relayCalls won't work for expensive operations Vulnerability Details With maxGasLimit assigned to more than typeuint32.max and relayCalls being called with gasLimit equal or greater than typeuint32.max + 1, it will revert because of the overflow at casting. Thi...

Incorrect _isAuthorized function logic

Lines of code Vulnerability details Summary The isAuthorized function in the CrossChainExecutorOptimism contract compares the message sender address to the address of the crossDomainMessenger contract, but it should be comparing the xDomainMessageSender value returned by the crossDomainMessenger...

Upgraded Q -> M from #402 [1670185822442]

Judge has assessed an item in Issue 402 as M risk. The relevant finding follows: Staked Gmx RewardTracker may retain allowances Summary: Both the configureGmxState function and the setContractContracts c, address contractAddress can be used to update the PirexGmx contract's stakedGmx storage...

Integer Overflow

Lines of code Vulnerability details Impact Potential integer overflow vulnerability in the relayCalls function. The function does not check for the maximum value of the gasLimit parameter, and if it is set to a value greater than the maxGasLimit, it will cause an integer overflow. Integer overflo...

Fail to verify the result of a low level function call

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. It fails to check properly if the call is successful because: 1 It does not check the return data; 2 it does not check if the target address is a valid contract. A contract that has no code will still...

DoS on relayCalls when the nonce variable reach type(uint256).max

Lines of code Vulnerability details Impact Denial of service on relayCalls functions when the nonce variable reach typeuint256.max Proof of Concept When the smart contracts start to be used, the variable in storage nonce will start to increment by 1, and since the nonce variable cannot be...

Potential reentrancy attack

Lines of code Vulnerability details Impact There is a potential of reentrancy attack in executeCalls in EthereumToArbitrumExecutor.sol since CallLib is making an external call with its executeCalls and we do not know the implementation of the contract that will be called eventually. The same issu...

[ZZ-004] During the deprecation period where both .eth registrar controllers are active, a crafted hack can be launched and cause the same malicious consequences of [ZZ-001] even if [ZZ-001] is properly fixed

Severity: High Status: Has not been reported Description, Specifically, according to the documentation, there will be a deprecation period that two types of .eth registrar controllers are active. Names can be registered as normal using the current .eth registrar controller. However, the new .eth...

Upgraded Q -> M from #265 [1669939411439]

Judge has assessed an item in Issue 265 as M risk. The relevant finding follows: ETH not updated in internal accounting of giant pools Giant pools have a feature of bringing ETH back to the giant pools from the vaults. The received ETH in not updated in internal accounting and therefore cannot be...

Relayed calls in EthereumToArbitrumRelayer can be replayed

Lines of code Vulnerability details Impact A relayed call can be processed multiple times allowing somebody to execute the user's call multiple times. The bridge allows arbitrary calls to contracts. Depending on the nature of the call, this could result in a loss of funds for the user. Proof of...

Reentranxcy in executeCalls()

Lines of code Vulnerability details Impact The contract first checks that the message sender is authorized and then marks the provided nonce as executed before calling the executeCalls function in CallLib. However, if CallLib.executeCalls calls back into this contract, the executeCalls function...

Upgraded Q -> M from #373 [1670018401792]

Judge has assessed an item in Issue 373 as M risk. The relevant finding follows: N2. ETH not accumulated in previewAccumulatedETH supposed to have accumulated += ... Although it is an external view function, depending on its usages, it may present more issues to the callers. --- The text was...

[ZZ-001] PARENT_CANNOT_CONTROL and CANNOT_CREATE_SUBDOMAIN fuses can be bypassed

Severity: High Status: Has been reported to and comfirmed by Jeff ENS team Report Time: 11/28/2022 12:31 AM EST Description The fuse constraints can be violated by a malicious owner of the parent node i.e., the hacker. There are two specific consequences the hacker can cause. Suppose the subnode...

Upgraded Q -> M from #205 [1669936745167]

Judge has assessed an item in Issue 205 as M risk. The relevant finding follows: L01 - No check if EOARepresentative or EOARepresentativeOfNodeRunner is an EOA or a smart contract Impact A smart contract can end up being assigned as a smartWalletRepresentative. Such smart contract might not have...

Upgraded Q -> M from #236 [1669938261828]

Judge has assessed an item in Issue 236 as M risk. The relevant finding follows: It is possible the DAO determine the amount of commission very high for its own benefit, so setting a range for determining the commission seems logical. Especially MAX commission should be pre- defined. --- The text...

Upgraded Q -> M from #121 [1669815281189]

Judge has assessed an item in Issue 121 as M risk. The relevant finding follows: Require - always false contracts/liquid-staking/LiquidStakingManager.sol:280 isNodeRunnerWhitelistednodeRunner comparing to itself requireisNodeRunnerWhitelistednodeRunner != isNodeRunnerWhitelistednodeRunner,...

Upgraded Q -> M from #38 [1669737057849]

Judge has assessed an item in Issue 38 as M risk. The relevant finding follows: 6. require check will always be false!! the require check will always be false so it the function will not do what it was designed to do and it will always revert Unnecessary update to same status...

Upgraded Q -> M from #16 [1669734774225]

Judge has assessed an item in Issue 16 as M risk. The relevant finding follows: AQ6: This function provides too much power to Dao, if the dao calls the function, then he can be the node runner of each smart wallet and then call withdrawETHForKnot to drain each smart wallet. --- The text was updat...

Slippage protection on AutoPxGmx::compound calls is insufficient and can result in MEV

Lines of code Vulnerability details Proof of Concept The compound method has the amountOutMinimum parameter, which basically serves as the slippage tolerance parameter. The problem is that everywhere in the code where compound is called, the value of amountOutMinimum is just 1 wei, which basicall...

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

Deposits and compounds will be frozen after a PirexGmx migration

Lines of code Vulnerability details Impact After a migration of the platform PirexGmx contract, the approval of the AutoPxGlp from the new PirexGmx contract will be zero. The same issue is here for the AutoPxGmx contract. Proof of Concept Even though the approval of gmxBaseReward formerly WETH fo...

The existence of the tokenID is not validated in distributeFees()

Lines of code Vulnerability details Impact Turnstile contract has distributeFees function which the Canto team/smart contract utilizes to distribute the fees to the tokenID's for the smart contract that is registered through register function. The existence of the tokenID's are checked both in...

distributeFees did not check if tokenId exist can lead to loss of asset

Lines of code Vulnerability details Impact distributeFees did not check if tokenId existed, can lead to loss of asset Proof of Concept inside Turnstile.sol file, the distributeFees function did not check if the tokenId exist or not. 148: function distributeFeesuint256 tokenId public onlyOwner...

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

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

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

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

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

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

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

compound could be used by uniswap stakers to maximize fees for AutoPxGmx users

Lines of code Vulnerability details Impact Anyone can call AutoPxGmx::compound. Hence a staker in the 10000 1% fee uniswap pool can call compound with that pool and take a larger fee from AutoPxGmx users maximizing their gains and griefing users. Proof of Concept fee chooses which uniswap pool to...

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

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

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

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

´userAccrue` rewards manipulation

Lines of code Vulnerability details Impact A flashloan can be used to set a huge last balance which later will accrue a huge reward. Proof of Concept Buy lots of a rewards-producing token, possibly by means of a flashloan. Call PirexRewards.userAccrue which sets u.lastBalance to this now very hig...

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