Vault can never fully be emptied

Lines of code Vulnerability details Impact Vault cannot be fully emptied Proof of Concept Whenever rewards are earned they are automatically locked into the the Aura Locker. Since that reward will then earn more rewards while locked, there will be more rewards to be collected when that lock is...

Withdrawing all funds at once to vault can be DoS attacked by frontrunning and locking dust

Lines of code Vulnerability details Impact All funds can be migrated withdrawn at once to the caller vault by using the BaseStrategy.withdrawToVault function which internally calls MyStrategy.withdrawAll. The latter function has the following check in place: MyStrategy.solL184-L187 require...

Yearn Curve vault can be removed without withdrawing all deposited tokens. Causing these token to be locked forever unless an owner is added this vault back.

Lines of code Vulnerability details Impact Yearn Curve vault can be removed without withdrawing all deposited tokens. Causing these token to be locked forever unless an owner is added this vault back. Proof of Concept function removeVaultaddress vault external onlyOwner requirevaultsvault !=...

VULNERABLE VERSION OF OPEN-ZEPPELIN CAN LEAD TO INITIALIZER() MALFUNCTION

Lines of code Vulnerability details VULNERABLE VERSION OF OPEN-ZEPPELIN CAN LEAD TO INITIALIZER MALFUNCTION Openzeppelin contracts are used, and as indicated in brownie-config.yaml, the version 3.4.0 of the openzeppelin-contracts-upgradeable dependency is used. The openzeppelin contracts from...

Wrong initial value of exitFees and EntryFees

Lines of code Vulnerability details Impact L49/53/159/167 - The exitFees and entryFees variables can only have values ​​between 1 and 10000, which represent 0.01% and 100%, but since it is not set in the constructor, the default value is 0, a value that is incorrect. Recommended Mitigation Steps ...

Fee-on-transfer support

Lines of code Vulnerability details Impact Every time transferFrom or transfer function in ERC20 standard is called there is a possibility that underlying smart contract did not transfer the exact amount entered. It is required to find out contract balance increase/de...

Upgraded Q -> M from 135 [1655579793739]

Judge has assessed an item in Issue 135 as Medium risk. The relevant finding follows: L send is not recommended for sending native token Since the introduction of transfer, it has typically been recommended by the security community because it helps guard against reentrancy attacks. This guidance...

Upgraded Q -> M from 72 [1655579867255]

Judge has assessed an item in Issue 72 as Medium risk. The relevant finding follows: Use SafeTransferLib for all ETH send With the exception of issueRefund, other transfer should allow forward all gas to finish the transaction. To prevent case of costing more than 23000 gas for transfer. Line1...

YearnCurveVaultOperator's depositETH can leave the remainder ETH funds frozen and unaccounted for, then utilized by another caller

Lines of code Vulnerability details depositETH effectively do not control the utilization of input token and can freeze WETH input funds in native ETH form on the contract balance when Yearn pool doesn't perform liquidity addition for any reason. Due to presence of the additional WETH - ETH step,...

Upgraded Q -> M from 243 [1655572117784]

Judge has assessed an item in Issue 243 as Medium risk. The relevant finding follows: Title: Using SafeERC20 library in ForgottenRunesWarriorsMinter.sol There are some token which are not implementing current ERC20 standard example: USDT, OmiseGo and BNB. Using SafeERC20 library will be nice to...

Upgraded Q -> M from 153 [1655579808802]

Judge has assessed an item in Issue 153 as Medium risk. The relevant finding follows: L03 Callvalue:amt"" should be used instead of send In ForgottenRunesWarriorsGuild.withdrawAll. transfer and send should be avoided because they take a hard dependency on gas costs by forwarding a fixed amount of...

The withdrawal safety check in _withdrawSome() seems unreasonable

Lines of code Vulnerability details Impact The withdrawal safety check in seems unreasonable. Proof of Concept I don’t understand why max = amount99.8% need to be confirmed. max should be larger than amount. And amount function withdrawSomeuint256 amount internal override returns uint256 uint256...

Upgraded Q -> M from 47 [1655579885825]

Judge has assessed an item in Issue 47 as Medium risk. The relevant finding follows: 4. Don't use payable.transfer/payable.send The use of payable.transfer is heavily frowned upon because it can lead to the locking of funds. The transfer call requires that the recipient is either an EOA account,...

TODO: Hardcode claim.account = address(this)?

Lines of code Vulnerability details Impact Why you still has a TODO in the final code? TODO: Hardcode claim.account = addressthis? It is not implemented yet. claim.account may be any value, which may break the claiming process or let user steal fund that intended to be used in MyStrategy to their...

Upgraded Q -> M from 91 [1655579814367]

Judge has assessed an item in Issue 91 as Medium risk. The relevant finding follows: use call instead of send affected code: 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 som...

Upgraded Q -> M from 270 [1655579826704]

Judge has assessed an item in Issue 270 as Medium risk. The relevant finding follows: Gas stipend for payable.send may be too low for contract wallets ETH withdrawals in both the minter and token contracts use payableaddress.send to transfer ether to the vault address. If the configured vault is ...

Upgraded Q -> M from 44 [1655579898351]

Judge has assessed an item in Issue 44 as Medium risk. The relevant finding follows: 1. Usage of legacy ETH transfer function Risk Low Impact Contract ForgottenRunesWarriors for withdrawing ETH to vault uses send function, which has a fixed gas stipend and can fail. The reason behind this is that...

_sendTokenToBribesProcessor() doesn't check bribesProcessor's address. Could cause permanent loss of fund

Lines of code Vulnerability details Impact In sendTokenToBribesProcessor, it sends tokens to bribesProcessor. It seems to be ok because claimBribesFromHiddenHand will confirm that bribesProcessor is not address0. sweepRewardToken also triggers sendTokenToBribesProcessor. But it doesn’t check...

If token == BADGER, the _handleRewardTransfer function does not work

Lines of code MyStrategy.solL405-L413 BaseStrategy.solL346-L353 Vault.solL396-L415 Vulnerability details Impact In MyStrategy.sol, when claiming bribes or sweeping reward tokens, the handleRewardTransfer function is called which calls sendToBadgerTree to send the amount of BADGER in the contract ...

Upgraded Q -> M from 143 [1655579880057]

Judge has assessed an item in Issue 143 as Medium risk. The relevant finding follows: L-04 safeTransferETH should perform simple ETH transfers and don’t forward 30k gas Link: Being a simple funds transfer, having a fallback of a WETH deposit, there should be no extra gas involved when potentially...

Reward token (auraBal) can be locked in the strategy

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. getRewardaddress account function of Aura Locker is an external function therefore can be called by anyone by passing in the address of strategy and transferring the rewards to the strategy. harvest...

Attacker can grief users from withdrawing their tokens and causing users to loose money

Lines of code Vulnerability details Impact balance of this contract of vault tokens in the require statement can stop users from withdrawing their tokens 2 issues: 1. vaultbalancebefore= balance of this contract from a vault tokens vaultamount=vaultbalancebefore- balance of vault right then when...

destroy function doesn't check if exitFees is set

Lines of code Vulnerability details Impact This issue might lead to 0 amountFees Proof of Concept The destroy function does not check if exitFees is set, therefore in a scenario wherein the owner does not set the value of exitFees, its default value would be 0. In line 264 we can see: uint256...

receive function is unrestricted

Lines of code Vulnerability details Impact The receive function has not placed any restriction which means if any user accidentally sends any ETH to this contract then it is permanently freezed until timelock decides to release it by creating and approving a new transaction on timelock which...

TimelockControllerEmergency: The sent ether may be locked in the OwnerProxy contract

Lines of code Vulnerability details Impact The call function in the TimelockControllerEmergency contract will send ether to the OwnerProxy contract, and the OwnerProxy contract will delegatecall the script contract. The two existing script contracts will neither use ether nor withdraw ether, whic...

attacker can lock all the auraBAL rewards in contract address forever and they won't be accessible

Lines of code Vulnerability details Impact auraBAL token is in protected tokens list, so it can't be transferred to bribeProcessor by using sweepRewardToken. function harvest is supposed to call LOCKER.getReward and then swap received auraBAL rewards and deposit them in LOCKER, but it only can do...

Functions in OperatorSripts.sol can be called by anyone

Lines of code Vulnerability details Impact The functions defined in OperatorScripts contract are external functions without any other access control. So anyone can add operators for OperatorResolver.getOpertor which is called in MixinOperatorResolver to use in callOperator, which is called when...

Owner can sweep any token

Lines of code Vulnerability details Impact Admin can sweep any token even if the token is in use by the contract. Ideally only non blacklisted tokens should be allowed by unlockTokens function function unlockTokensIERC20 token external override onlyOwner uint256 amount = token.balanceOfaddressthi...

DOS on operation execution

Lines of code Vulnerability details Impact A malicious proposer can keep on cancelling all pending operations so that none of the transactions get executed. Admin also has no way to remove the malicious proposer Proof of Concept 1. Proposer A calls schedule function to schedule an operation 2...

no slippage check

Lines of code Vulnerability details in the function swapAndAddLiquidity it makes a call swapExactTokensForTokens with slippage hard coded to 1 this could lead to the user receiving much less tokens than expected due to being frontrun / sandwiched which will result in a loss of funds recommend...

There is no check that in setBribesProcessor() the value of newBribesProcessor is not 0x0, fund will be lost or locked if by mistake value set to 0x0

Lines of code Vulnerability details Impact Fund can be lost if the value of bribesProcessor set to 0x0 address and there is no check in the setBribesProcessor to prevent it. sendTokenToBribesProcessor sends bribes to bribesProcessor and there is no check there too. so if by mistake the value of...

All withdrawal functionality is paused when contract is paused

Lines of code Vulnerability details Impact When the strategy contract is paused, all withdrawal functionality will be paused. Based on the comments in MyStrategy.sol and baseStrategy.sol, withdrawToVault should not be affected by the pause functionality. This is not the case due to the...

fund lose because of the direct funds transfer to vault address and cause big balance() to totalSupply() ratio and cause big division error in _mintSharesFor() (this is in previous contest scope)

Lines of code Vulnerability details Impact Attacker can cause balance / totalSupply ratio to go as high as he want and then because of rounding error in mintSharesFor lower amount of share would be mint for users. if totalSupply is 0 attacker can directly transfer tokens to contract address and...

attacker can call sweepRewardToken() when bribesProcessor==0 and reward funds will be lost because there is no check in sweepRewardToken() and _handleRewardTransfer() and _sendTokenToBribesProcessor()

Lines of code Vulnerability details Impact If the value of bribesProcessor was 0x0 the default is 0x0 and governance can set to 0x0 then attacker can call sweepRewardToken make contract to send his total balance in attacker specified token to 0x0 address. Proof of Concept the default value of...

Missing reentrancy protections

Lines of code Vulnerability details Impact The files below contain both deposit and withdraw functions which seem re-entrable at the point of calls that transfer tokens. The functions do not fully follow a checks-effects-interactions pattern, thus they can be re-entered multiple times. Depending ...

reentrancy by _harvest() and _deposit() code in the middle of claimBribesFromHiddenHand() external calls to tokens and change balances of AURA token so claimBribesFromHiddenHand() make wrong transfers and fund would be lost

Lines of code Vulnerability details Impact Function claimBribesFromHiddenHand makes some external calls to token lists which fetches from hiddenHandDistributor.rewards if AURA was on of those tokens and also one of those tokens were malicious or made some external call then it's possible to reent...

Update initializer modifier to prevent reentrancy during initialization

Lines of code Vulnerability details The solution uses: OpenZeppelin/[email protected] These dependencies have a known high severity vulnerability: Which makes these contracts vulnerable: Recommended Mitigation Steps Upgrade @openzeppelin/contracts-upgradeable to version 4.4...

Lack of Two-Step Process for Critical Operations

Lines of code Vulnerability details Impact This function transfers/renounce the ownership of the contract in a single step. There is no way to reverse a one-step transfer of ownership to an address without an owner. This would not be the case if ownership were transferred through a two-step proce...

amountAMin and amountBMin set to 1

Lines of code Vulnerability details in the function swapAndAddLiquidity it makes a call addLiquidity with amountAMin and amountBMin hard coded to 1 recommend specifying a proper amountAMin and amountBMin rather than 1 --- The text was updated successfully, but these errors were encountered: All...

Update initializer modifier to prevent reentrancy during initialization

Lines of code Vulnerability details Proof of Concept The code uses: @openzeppelin-contracts-upgradeable=OpenZeppelin/[email protected]/contracts/ This dependency have a known high severity vulnerability: Which makes the main contract in this audit "MyStrategy" vulnerable...

Malicious Owner can steal all user funds

Lines of code Vulnerability details Submitting as med risk because it would require malicious multisig, but there should never be absolute trust in any party especially when there's no reason fees would ever need to be that high anyways Impact Owner steals all of user funds Proof of Concept...

Gitignore too low

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. Too short gitignore Tools Used Recommended...

Deposit and mint function will be rendered useless for users who are depositing using eth since balances will never be finalised

Lines of code Vulnerability details The penultimate function in NotionalV2.batchBalanceAndTradeAction calculates the withdrawals and finalises the balance as mentioned : but the following calculation will revert on an underflow since balanceState.netAssetTransferInternalPrecision is set to 0 in...

Posible UUPSUpgradeable attack

Lines of code Vulnerability details Impact Missing implementation of the modifier Summary: initializer modifier is found in wfCashBase.sol but none of the OZ Initializable or AAVETokenV2Mintable.sol are included in wfCashBase or any of its related files. Details: I couldn’t find the modifier to b...

Approve Returned Value Not Validated

Lines of code Vulnerability details Proof-of-Concept The approve function attempts to performs an ERC20.approve call, but does not check if the returned value is true Succeed or false Failed. Some tokens do not revert if the approval failed but return false instead. / @dev Approve the given...

Did Not Enforce fCash To Be A Component Of SetToken Before Minting

Lines of code Vulnerability details Proof-of-Concept Assume that the manager decided to add a fCash position called "Wrapped fDAI @ 10 October 2022", which will mature at 10 October 2022, to the SetToken. To do so, the manager will call the NotionalTradeModule.mintFCashPosition function. The...

Round down in previewWithdraw() may result in withdrawing asset using zero share.

Lines of code Vulnerability details Impact In wfCashERC4626.previewWithdraw function, when fCash has matured, shares is calculated using convertToShares. But convertToShares function rounded down in division. This may lead to the case that user can use zero share to withdraw asset. It has been...

Immature fCash positions should not be left behind when removing the NotionalTradeModule

Lines of code Vulnerability details function removeModule external override onlyValidAndInitializedSetISetTokenmsg.sender ISetToken setToken = ISetTokenmsg.sender; // Redeem matured positions prior to any removal action redeemMaturedPositionssetToken; // Try if unregister exists on any of the...

Too many setToken modules may cause gas limit error

Lines of code Vulnerability details Impact Too many setToken modules may cause gas limit error. Has these impacts: 1. These setToken can't be initialized in NotionalTradeModule 2. Cannot remove any module from that setToken Proof of Concept / @dev MANGER ONLY: Initialize given SetToken with initi...

Residual Allowance Might Allow Tokens In SetToken To Be Stolen

Lines of code Vulnerability details Proof-of-Concept Whenever mintFCashPosition function is called to mint new fCash position, the contract will call the approve function to set the allowance to maxSendAmount so that the fCash Wrapper contact can pull the payment tokens from the SetToken contract...