Not all ERC20 tokens return boolean on transfer

Lines of code Vulnerability details Impact Some ERC20 tokens do not conform to the standard of returning a boolean when transfer is called. If one of these tokens is included as a reward token, the withdraw function will be irrevocably broken, and users won't be able to collect their reward or...

Yield source does not correctly calculate share conversions

Lines of code Vulnerability details The aTokens’ value is pegged to the value of the corresponding supplied asset at a 1:1 ratio and can be safely stored, transferred or traded. All yield collected by the aTokens' reserves are distributed to aToken holders directly by continuously increasing thei...

Inactive skipped assets can be drained from the index

Lines of code Vulnerability details Impact If an index has any inactive assets with the role SKIPPEDASSETROLE, a user can repeatedly deposit and withdraw assets, always getting the skipped asset without having to deposit any Proof of Concept During minting, any asset that has the 'skipped' role i...

Users wont be able to withdraw staked tokens since vesting function is called differently in interface and implementation

Lines of code Vulnerability details Impact Users can't withdraw their funds Proof of Concept When withdrawing in staking the contract will send funds to vesting using setupVesting. This is defined in the vesting interface. On the other hand, the vesting implementation calls this function vest so...

IVesting(vesting).setupVesting is not implemented in StakedCitadelVester.sol.

Lines of code Vulnerability details Impact When doing withdraw or withdrawAll in StakedCitadel.sol, it will call the internal function withdraw. The function then transfers tokens to the vesting contract, which should be StakedCitadelVester.sol. However, IVestingvesting.setupVesting is not...

User withdrawal can be stuck

Lines of code Vulnerability details Impact Any transfer will lead to updation of cooldown period for the recipient. This can become a problem if recipient wished to withdraw some amount using unstake. Proof of Concept 1. User A wished to withdraw amount 100 2. User A is pending 1 more day where...

[WP-H11] lender may not be able to get back their funds, due to improper handling of potential loss of strategy

Lines of code Vulnerability details uint256 notBorrowed = pooledCLConstantsid.borrowLimit.subPOOLEDCREDITLINE.getPrincipalid; uint256 notBorrowedInShares = IYieldstrategy.getSharesForTokensnotBorrowed, borrowAsset; uint256 sharesHeld = pooledCLVariablesid.sharesHeld; requiresharesHeld != 0,...

[WP-M10] Wrong formula of getSharesForAmount() can potentially cause fund loss when being used to calculate the shares to be used in withdraw()

Lines of code Vulnerability details In Collateral, the getter functions getAmountForShares and getSharesForAmount is using totalAssets instead of strategyController.totalValue, making the results can be different than the actual shares amount needed to withdraw a certain amount of baseToken and t...

LiquidityFarming.withdraw permanently freezes any unpaidRewards left

Lines of code Vulnerability details Impact Remainder nft.unpaidRewards are lost and cannot be retrieved after LiquidityFarming.withdraw. I.e. it is not possible to extractRewards for unpaidRewards later if withdraw being called when balance wasn't sufficient to fulfil the full withdraw of the...

[WP-H4] Deleting nft Info can cause users' nft.unpaidRewards to be permanently erased

Lines of code Vulnerability details function withdrawuint256 nftId, address payable to external whenNotPaused nonReentrant address msgSender = msgSender; uint256 nftsStakedLength = nftIdsStakedmsgSender.length; uint256 index; for index = 0; index...

Using transfer instead of safeTransfer allows attacker to steal all staked tokens

Lines of code Vulnerability details Impact An attacker can drain all staked tickets. Proof of Concept In the withdrawDelegationToStake function the user can transfer from one of his delegations to the TWABDelegator contract and receive vault tokens he can then unstake to get his tickets back...

QA Report

Low and non-critical bugs wrong implementation of ERC4626RouterBase's withdraw function In the interface we can see the function description: / @notice withdraw amount from an ERC4626 vault. @param vault The ERC4626 vault to withdraw assets from. @param to The destination of assets. @param amount...

ERC4626.mint() doesn't mint the correct amount

Lines of code Vulnerability details Impact The ERC4626.mint function doesn't mint the correct amount of tokens. Instead of minting amount number of tokens, it should mint shares number of tokens. Since the user doesn't receive the correct amount of tokens I'd rate this issue "HIGH". Proof of...

Griefing attack at VUSD withdraw queue is possible

Lines of code Vulnerability details Impact A malicious user can make lots of withdrawal requests to fill up the queue, making VUSD withdrawals unreachable for all other users Proof of Concept There is no control of the size or number of the withdrawal requests, and VUSD will burn even 1 wei amoun...

ERC4626RouterBase.withdraw should use a **max** shares out check

Lines of code Vulnerability details Impact The docs/video say ERC4626RouterBase.sol is in scope as its part of TurboRouter The ERC4626RouterBase.withdraw function withdraws the asset amount parameter by burning shares. function withdraw IERC4626 vault, address to, uint256 amount, uint256...

Use of IERC20.transfer() instead of SafeERC20.safeTransfer()

Lines of code Vulnerability details Impact There are multiple external calls to IERC20.transfer that suppose to transfer out tokens from the system. However, there are tokens like USDT that don't return any return value in their implementation of the transfer function, so calling IERC20.transfer ...

All Tokens Can Be Stolen From Shelter Contract

Lines of code Vulnerability details Impact function withdrawIERC20 token, address to external override requireactivatedtoken != 0 && activatedtoken + GRACEPERIOD block.timestamp, "shelter not activated"; uint256 amount = savedTokenstoken client.shareOftoken, msg.sender / client.totalSharetoken;...

[WP-H14] ConvexStakingWrapper, StakingRewards Wrong implementation will send concur rewards to the wrong receiver

Lines of code Vulnerability details UserInfo storage user = userInfopidmsgSender; updatePoolpid; ifuser.amount 0 uint pending = user.amount pool.accConcurPerShare / concurShareMultiplier - user.rewardDebt; if pending 0 safeConcurTransferrecipient, pending; ConvexStakingWrapper, StakingRewards is...

missing check on claimed token

Lines of code Vulnerability details Impact the withdraw can be used by user to claim a token, however this function didnt check of the user had already claimed or not, therefore setting claimedtokento = true; is useless, therefore the user can claimed in multiple times, till the contract is...

Re-entrancy vulnerabilities

Lines of code Vulnerability details Impact Function claimRewards in ConcurRewardPool should be re-entrancy protected or first nullify the reward before sending it, otherwise, if any token contains a transfer callback hook, users can claim the same rewards multiple times, by re-entering the functi...