Upgraded Q -> 3 from #264 [1691857350267]

Judge has assessed an item in Issue 264 as 3 risk. The relevant finding follows: Let's see how it can be exploited. You can add this test to Withdraw.t.sol and run with forge test -vv --match-contract VaultWithdrawTest --match-test testWithdrawAllAssetsForHalfShares: function...

TEST PR 1093 - edited

Lines of code L1 Vulnerability details edited Assessed type call/delegatecall --- The text was updated successfully, but these errors were encountered: All reactions...

Governor relay() functions can be used to bypass voting an directly run critical SecurityCouncilManager functions

Lines of code https://github.com/ArbitrumFoundation/governance/blob/c18de53820c505fc459f766c1b224810eaeaabc5/src/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.sol...

Delegated Votes Blocking Delegator Undelegation

Lines of code Vulnerability details Impact Once a delegator has delegated their votes to a delegatee, and the delegatee employs those votes in an ongoing proposal, the delegator loses the ability to undelegate their votes. VotingEscrow::delegate is used to delegate user A's votes to User B. Once...

User don't have to deposit for a week into the market to get his weekly reward from the LendingLedger

Lines of code Vulnerability details Impact In the LendingLedger contract, a user is rewarded with CANTO tokens depending on how long he has his deposit in the market. Rewards are distributed for each week during which the deposit was inside the market. However, the user can cheat this condition...

otherCohort() is not returning different value when the Election has ended.

Lines of code Vulnerability details Impact In SecurityCouncilNomineeElectionGovernor.sol when the otherCohort function is called, it should return the otherCohort. But it is not the case because it returns the cohort at electionIndexToCohort at electionCount - 2 which is only updated when another...

createLock, increaseAmount int128(int256(_value)) unsafe downcast can lead to asset loss

Lines of code Vulnerability details Impact Suppose users deposit more than typeint128.max value through createLock and increaseAmount, they may get less voting power and can't get the assets back. Proof of Concept The VotingEscrow.sol is forked from FIATDAO, but it seems don't consider a MEDIUM...

Anyone can change the members of Security Council

Lines of code Vulnerability details Impact Anyone can change the members of security council by calling the function perform in the contract SecurityCouncilMemberSyncAction.sol as the function is open to all. Proof of Concept uint256 updateNonce = getUpdateNoncesecurityCouncil; if nonce =...

User can claim most of the rewards for a lending market by depositing cNote for just 2 blocks / epoch

Lines of code Vulnerability details Impact For a user to receive rewards for supplying cNote in a lending market LM, he only needs to have supplied the cNote at the end of an epoch. Users staking for the whole duration of an epoch get 0 benefits, compared to users who supply only at the end of th...

critical functions should emit an event when they called

Lines of code Vulnerability details Impact some critical functions should emit an even when they called because it contains important information and an event should be emitted, some functions in this contest did not emit an event and this may led to unwanted behavior in future functions like cla...

Undelegation logic doesn't work as expected, duo to that an owner of a lock would be enforced to extend their lock time for another 5 years.

Lines of code Vulnerability details Impact Undelegation logic doesn't work as expected, duo to that an owner of a lock would be enforced to reset his lock time for another 5 years through the function increaseAmount in order to successfully undelegate. Proof of Concept There are few differences...

Anyone can complete the Rng relay auction

Lines of code Vulnerability details Impact There's no access restriction on rngComplete which allows anyone to execute this function and complete the auction. This can lead to unexpected behavior or potential DOS attack where a user completes the auction. Tools Used Manual Analysis Recommended...

Wrong Implementation of Continuous Gradual Dutch Auction

Lines of code Vulnerability details Impact Breaks the core functionality of the Liquidation Pair contract. Usage of wrong formula for calculation of Continuous Gradual Dutch Auction results in wrong calculation of purchase price which is basically used to find the swapAmountIn during liquidations...

Unchecked Call Return Value

Lines of code Vulnerability details Impact The functions do not check the return value of low-level calls. This can lock Ether in the contract if the call fails or may compromise the contract if the ownership is being changed. The following calls were detected without return value validations -...

Potential Near-Zero Scenarios for purchasePrice in the Continuous Gradual Dutch Auction

Lines of code Vulnerability details Impact The Continuous Gradual Dutch Auction CGDA model has potential scenarios where the purchasePrice for an amount of tokens could approach near-zero values. This is influenced mainly by two factors: emissionRate and timeSinceLastAuctionStart. If either one o...

cleanup() does not properly handle debt repayment

Lines of code Vulnerability details Impact The cleanup... function in the PositionManager.sol contract is used to deposit remaining users assets back to ROE, repaying debt if any. However the users debt will not be repaid if the user has debt leading to loss of funds for the lenders and the proje...

The RngRelayAuction::rngComplete() function can be called by anyone (malicious actor) causing that the draw will be closed using a malicious random number

Lines of code Vulnerability details Impact The rngComplete function is called by the RngAuctionRelayerDirect contract or the RngAuctionRelayerRemoteOwner contract. The rngComplete function receives the RNG results from the rng auction. The problem here is that the rngComplete function can be call...

Missing access control in RngRelayAuction::rngComplete()

Lines of code Vulnerability details For a draw auction to complete, a bot must relay a completed RNG result to the Prize Pool. This is done by calling rngComplete and the data must originate from the relayer. However the rngComplete function is missing a check that the msg.sender is the relayer s...

removeFromAllTicks() withdraws all tick assets before deposit and withdraw re-deposit them creates a reentrancy attacks.

Lines of code Vulnerability details Impact reentrancy attacks can result to stolen funds Proof of Concept The key issue is that removeFromAllTicks calls removeFromTickindex in a loop, which calls lendingPool.withdraw and tr.withdraw. These external calls could trigger a reentrant call back into t...

Owner of _liquidationPair can add malicious liquidation source and insufficient input parameter validations

Lines of code Vulnerability details Impact Malicious liquidationPair owner can deploy liquidationPair using malicious sourceliquidation source that the pair will use and other insufficient input validations that can put users' funds at risk. File: src/LiquidationPairFactory.sol function createPai...

Cross-chain replay attacks are possible

Lines of code Vulnerability details Impact In MarketERC20.sol we have permit function: function permit / bool asset, // 1 = asset, 0 = collateral address owner, address spender, uint256 value, uint256 deadline, uint8 v, bytes32 r, ...

Reentrancy vulnerability in SGLLendingCommon._removeCollateral

Lines of code Vulnerability details Impact This vulnerability could allow an attacker to withdraw collateral from the SGLLendingCommon contract without actually removing it. This could result in a loss of funds for the lender. Proof of Concept The SGLLendingCommon.removeCollateral function is...

ARBTriCryptoOracle is prone to manipulation

Lines of code Vulnerability details Impact ARBTriCryptoOracle is used to determine price of LP token of tricrypto USDT, WBTC, WETH on arbitrum. This pool is susceptible to re-entrancy due to bug in vyper 0.2.15. and hence getvirtualprice can be manipulated which is used for pricing LP tokens. Pro...

BigBang liquidations causes YieldBox-tokens to be locked in contract

Lines of code Vulnerability details Impact When a position gets liquidated in BigBang the contract will receive YieldBox-assetId-tokens of which some are sent to the liquidator and penrose-fee-receiver. The rest will get stuck in the contract and cannot be claimed as fees in refreshPenroseFees...

Incorrect parameter for getCallerReward might return 0 reward despite insolvency

Lines of code Vulnerability details Impact The calculation of the caller reward uses an incorrect value. If the exchangeRate remains the same but a lot of interest accrues, then there will be no liquidation reward. Without a liquidation reward borrowing positions will not get liquidated and incur...

Setting debtStartPoint > 0 breaks many BigBang actions

Lines of code Vulnerability details Impact If BigBang.debtStartPoint is set to a value 0, many core features will break, e.g. deposits of collateral will be possible, but removal not, which would effectively lock collateral inside the contract. Proof of Concept BigBang.getDebtRate uses the variab...

borrowInternal() of BaseTOFTMarketModule.sol has phantom permit functions

Lines of code Vulnerability details Impact A malicious actor could steal funds from a User who has already done his first deposit. Proof of Concept Consider the case where attacker uses a token with phantom permit function as collateral, the most famous ones being WETH, BNB, HEX etc. Let’s consid...

There is no decrease for the share allowance from _addCollateral when share passed zero

Lines of code Vulnerability details Impact When calling addCollateral, and if the share passed as zero, it is calculated based on the passed amount. However, this happens after allowanceBorrow was already called in addCollateral. So, deduction never occur for the share. Eventually, the borrow...

Cannot unwrap token after recovering through wrapping

Lines of code Vulnerability details Impact Cannot unwrap token after recovering through wrapping. Proof of Concept FollowNFTunwrap checks if followerProfileId for the token is not 0: if followDataByFollowTokenIdfollowTokenId.followerProfileId == 0 revert NotFollowing; while after recovering token...

Inaccurate implementation of ECDSA creates signature malleability

Lines of code Vulnerability details Impact The ecrecover function is an inherent cryptographic function within Solidity which enables the retrieval of the signer's address messages which have been signed with their private key. Of course, this is very important when it comes to verifying the...

Incorrect liquidity calculations if snapshot errors occur

Lines of code Vulnerability details Impact incorrect liquidity values that undermine the risk checks and constraints of the protocol Proof of Concept getHypotheticalAccountLiquidityInternal does not handle snapshot errors properly. Specifically: It calls MToken.getAccountSnapshot to get the...

EIP-712 typehash is incorrect for several functions in MetaTxLib

Lines of code Vulnerability details Bug Description In LensHub.sol, the second parameter of setProfileMetadataURIWithSig is declared as metadataURI: LensHub.solL119-L123 function setProfileMetadataURIWithSig uint256 profileId, string calldata metadataURI, Types.EIP712Signature calldata signature...

FollowNFT storage collision

Lines of code Vulnerability details Impact The lastFollowTokenId of FollowNFT contract has a storage collision. V2.0 storage layout: | lastFollowTokenId | uint128 | 17 | 0 | 16 | contracts/FollowNFT.sol:FollowNFT | | followerCount | uint128 | 17 | 16 | 16 | contracts/FollowNFT.sol:FollowNFT |...

Potential Stale Data Due to Inadequate Round Validation

Lines of code Vulnerability details Impact Even tough the dev is checking if the data is stale or not some valid prices could still be considered as invalid because of this line: answeredInRound == roundId, sometimes the answeredInRound might be greater than roundId and would still be valid but i...

Missing check of how recent the price is can lead to stale price being used in the protocol

Lines of code Vulnerability details Impact In the ChainlinkOracle.sol file, in the function getChainlinkPriceAggregatorV3Interface feed, there is the check requireupdatedAt != 0, "Round is in incompleted state";. However, there is no check to see that the price is recent and acceptable. If there ...

Malicious actors could pass in huge arrays to disrupt service availability (DOS) in the updateVotingPower function

Lines of code Vulnerability details Impact The contract is stopped from being usable by legitimate users if the attacker repeatedly spammed large arrays Proof of Concept The updateVotingPower function currently only checks that the array is = 50: While 50 may be a reasonable limit for normal usag...

Malicious user can create an harmful proposal and execute it by setting a very low quorum .

Lines of code Vulnerability details Impact Malicious user can create an harmful proposal and execute it by setting a very low quorum . Which can lead to very bad consequences . Proof of Concept When creating a proposal, quorum is calculation logic looks like this : // get the quorum requirement f...

Users with 0 votes can vote .

Lines of code Vulnerability details Impact Users with 0 votes can vote , which can lead to unwanted scenarios . Proof of Concept The function vote in CoreVoting.sol looks like this : function vote address memory votingVaults, bytes memory extraVaultData, uint256 proposalId, Ballot ballot public...

Missing Validation which could cause Underflow Error

Lines of code Vulnerability details Impact Two instances of Missing Validation which could cause Underflow Error are spotted at L194 and L593 of NFTBoostVault.sol contract, from L194 there is no check to validate that oldDelegateeVotes is greater than registration.latestVotingPower and from L593...

GSC will lose allowance if it tries to lower a spender's approval

Lines of code Vulnerability details Impact GSC will lose allowance if it tries to lower a spender's approval. Proof of Concept When gsc update a spender's approval, the gscAllowance will always decrease by amount: function gscApprove address token, address spender, uint256 amount external...

Not all profiles can be migrated to V2 profile

Lines of code Vulnerability details Impact There seem to be some differences in the validation logic that makes the V1 to V2 conversion for some profiles impossible. Proof of Concept // SPDX-License-Identifier: UNLICENCED pragma solidity =0.8.4; contract HandleMigration uint256 internal constant...

THIS IS A TEST

Lines of code L1 Vulnerability details TESTING REFACTOR Assessed type Context --- The text was updated successfully, but these errors were encountered: All reactions...

[H] Users can lose their funds if the transaction reverts on destination chain

Lines of code Vulnerability details Impact The transmitSendToken function in the contract facilitates cross-chain token transfers. For contracts involving ERC20 tokens it is also expected that, should a contract call fail, one's tokens are not lost/transferred elsewhere. However, the function doe...

All co-signers pay during executeMultisigProposal

Lines of code Vulnerability details Impact The executeMultisigProposal will receive payment from all cosigners when only one payment of nativeValue amount is required. Proof of Concept The executeMultisigProposal requires native payment and therefore requires the caller to pay an amount equivalen...

M-07 Unmitigated

Lines of code Vulnerability details The fix addresses the scenarios when collaterals are removed between the crafting of the minAmountsOut list and the submission of the transaction. Then, we will have amounts.length minAmountOuts.length, meaning that the following line causes a revert: if...

Anyone can mint to themselves type(uint96).max if _isVaultCollateralized() returns true

Lines of code Vulnerability details Impact There is no check that ensures the caller to mint is a trusted one. Moreover, there is a flaw which lets anyone to mint typeuint96.max number of shares Proof of Concept First, the mint function does not implement any check for the caller to be someone wi...

The _currentExchangeRate of the Vault contract can't increase, and always be lower than or equal to _assetUnit

Lines of code Vulnerability details Impact The currentExchangeRate of the Vault contract can not increase, and always be lower than or equal to assetUnit. Therefore, when the vault is undercollateralized currentExchangeRate assetUnit, it can't be further collateralized. Proof of concept function...

Users with DEPLOY permission can grief each other through CREATE2

Lines of code Vulnerability details Bug Description In ERC725XCore.sol, the deployCreate2 function uses Openzeppelin's Create2.deploy to deploy new contracts: ERC725XCore.solL253-L267 function deployCreate2 uint256 value, bytes memory creationCode internal virtual returns bytes memory newContract...

Missing External Transfer Function In Vault

Lines of code Vulnerability details Impact Balances of TwabController for a vault can not be transferred. Proof of Concept The Vault implements an internal transfer function meant to be used to transfer balances within the TwabController: function transferaddress from, address to, uint256 shares...

Potential for Permanent Lock of Funds in NounsDAOExecutorV2 Contract

Lines of code Vulnerability details Impact The NounsDAOExecutorV2 contract has a potential vulnerability where Ether or ERC20 tokens could become permanently locked in the contract. This could occur if funds are sent to the contract by someone other than the admin, or if the admin loses access to...