OverlayToken.burn function could burn tokens of any user

Handle xYrYuYx Vulnerability details Impact The burner could burn any amount of tokens of any user. This is not good solution of burn Tools Used Manual Recommended Mitigation Steps Update burn function for only owner can burn his tokens. Now, ovl.burn function is used in OverlayV1OVLCollateral.so...

transferMint and transferBurn don’t update totalSupply

Handle xYrYuYx Vulnerability details Impact In transferMint function, new tokens will be minted, but this does not increase total supply. In transferBurn function, some tokens will be burned, but did not decrease total supply. Tools Used Manual Recommended Mitigation Steps Update totalSupply in...

borrowFee can be completly bypassed while minting

Handle hack3r-0m Vulnerability details uint256 totalCredit = cdp.totalCredit; retrieves total credit of caller if totalCredit amount // user pays the fee ... else cdp.totalCredit = totalCredit.subamount; here amount is caller amount and = totalCredit then the user can skip paying fees everytime...

_transferBurn does not actually burn tokens (decrease supply)

Handle harleythedog Vulnerability details Impact The implementation of transferBurn in ovl/OverlayToken.sol does not actually burn any tokens since totalSupply is not decreased see the implementation for burn for reference of what should be done. The transferBurn function is a helper function tha...

_transferMint does not actually mint tokens (increase supply)

Handle harleythedog Vulnerability details Impact The implementation of transferMint in ovl/OverlayToken.sol does not actually mint any tokens since totalSupply is not increased see the implementation of mint for reference of what should be done. The transferMint function is a helper function that...

onPriceCheck revert on every transaction

Handle jonah1005 Vulnerability details Impact onPriceCheck in the Alchemist.sol does not implement correctly. modifier onPriceCheck if pegMinimum 0 require ICurveTokenaddresstoken.getvirtualprice pegMinimum, 'off peg limitation' ; ; There's no getvirtualprice in stable coin contracts. There's no...

function distribute() Allow Certain Users Transfer Third Persons Token aka Users'Wallet into Contract

Handle Meta0xNull Vulnerability details Impact function distributeToTransmuteruint256 amount internal token.approvetransmuter, amount; In Alchemist.sol, here is where the Token Approved. function distributeaddress origin, uint256 amount public onlyWhitelisted runPhasedDistribution...

CDP.sol update overwrites user's credit on every positive increment

Handle harleythedog Vulnerability details Impact Within CDP.sol there is a function called update. This function slowly decreases the debt of a position as yield is earned, until the debt is fully paid off, and the idea is then that the credit should begin incrementing as more yield is accumulate...

YaxisVaultAdapter.sol#withdraw() will most certainly fail

Handle WatchPug Vulnerability details The actual token withdrawn from vault.withdraw will most certainly less than the amount, due to precision loss in tokensToShares and vault.withdraw. As a result, IDetailedERC20token.safeTransferrecipient, amount will revert due to insufficant balance. Based o...

lowerHasMinted() Allow Minter to Mint Unlimited AlToken

Handle Meta0xNull Vulnerability details Impact uint256 total = amount.addhasMintedmsg.sender; requiretotal Tools Used Manual Review Recommended Mitigation Steps Remove function lowerHasMinted --- The text was updated successfully, but these errors were encountered: All reactions...

Transmuter yield can be gamed by only staking when yield is distributed

Handle cmichel Vulnerability details The Transmuter.distribute function distributes the yield to the buffer which is then distributed to all stakers over a TRANSMUTATIONPERIOD, see runPhasedDistribution and updateAccount. If the elapsed time from the last phase distribution is greater than the...

ERC20.transfer return value is ignored

Handle pants Vulnerability details ERC20 transfer and transferFrom return values are ignored. There is no whitelist, therefore users can use ERC20 tokens that transfer/transferFrom returns False instead of revert. for example: YaxisVoteProxy.sol line 55 MetaVault.sol line 579 MockPickleJar.sol li...

Does not check uniqueness of ShareHolder

Handle hack3r-0m Vulnerability details does not check if there is already a shareholder before creating a new shareholder. this will cause an issue in findShareHolder since it will return the first shareholder in the array while there are more than one. --- The text was updated successfully, but...

function mint() No Tokens Will be Transfer to _recipient

Handle Meta0xNull Vulnerability details Impact mintrecipient, amount; At the end of mint, mint seems like responsible to transfer tokens to recipient but there is no code no function of mint in this contract. Thus No Tokens Will be Transfer to recipient. recipient who do not Receive Their Tokens...

NestedFactory: Ensure zero msg.value if transferring from user and inputToken is not ETH

Handle GreyArt Vulnerability details Impact A user that mistakenly calls either create or addToken with WETH or another ERC20 as the input token, but includes native ETH with the function call will have his native ETH permanently locked in the contract. Recommended Mitigation Steps It is best to...

ERC1155Supply vulnerability in OpenZeppelin Contracts

Handle defsec Vulnerability details Impact When ERC1155 tokens are minted, a callback is invoked on the receiver of those tokens, as required by the spec. When including the ERC1155Supply extension, total supply is not updated until after the callback, thus during the callback the reported total...

Ensure on-chain that cache is synced

Handle GreyArt Vulnerability details Impact Currently, many core operations like NestedFactory.create, NestedFactory.swapTokenForTokens are dependent on the assumption that the cache is synced before these functions are executed however this may not necessarily be the case. Proof of Concept 1...

NestedFactory.sol#_submitInOrders() Wrong implementation cause users to be overcharged

Handle WatchPug Vulnerability details When executing orders, the actual amountSpent + feesAmount can be lower than inputTokenAmount, the unspent amount should be returned to the user. However, in the current implementation, the unspent amount will be taken as part of the fee. function...

Missing Emergency Pause Check

Handle defsec Vulnerability details Impact During the manual code review, It has been observed that minting progress is not checked when the contract is emergency paused. This can cause misfunctionality and unlocking user funds during the emergency pausing. Proof of Concept 1- Navigate to ""...

Prevent Minting During Emergency Exit

Handle TimmyToes Vulnerability details Impact Potential increased financial loss during security incident. Proof of Concept Consider a critical incident where a vault is being drained or in danger of being drained due to a vulnerability within the vault or its strategies. At this stage, you want ...

isResolverCached() will always return false after removing operator

Handle GreyArt Vulnerability details Impact While there is no loss of funds, removing an operator will cause the cache functionality to be permanently broken. If there was a function that had a modifier which requires the cache to be synced before the function can be called, it would not be...

importOperators() should be run atomically with rebuildCaches() to make sure addressCache is up-to-date with the new operators

Handle WatchPug Vulnerability details Given that importOperators will change operators, and addressCache will not be updated until rebuildCaches is called separately. To ensure addressCache is up-to-date, importOperators should be run atomically with rebuildCaches. Recommendation Consider changin...

FeeSplitter: No sanity check to prevent shareholder from being added twice.

Handle GreyArt Vulnerability details Impact It is possible for duplicate shareholders to be added. These shareholders will get more than intended when sendFee is called. Recommended Mitigation Steps Ensure that the accounts array is sorted in setShareholders. for uint256 i = 0; i 0 requireaccount...

Passing multiple ETH deposits in orders array will use the same msg.value many times

Handle hyh Vulnerability details Impact Contract holdings can be emptied as malicious user will do deposit/withdraw to extract value. This is possible because after transferInputTokens system uses contract balance for user's operations, assuming that equivalent value was transferred. Proof of...

MixinOperatorResolver.sol#isResolverCached() become malfunctioning when an operator is removed

Handle WatchPug Vulnerability details When an operator is removed and rebuildCache is called, isResolverCached should return true. It returns false in the current implemenbtation. /// @notice Check the state of addressCache function isResolverCached external view returns bool bytes32 memory...

FeeSplitter: Unbounded number of shareholders can cause DOS

Handle GreyArt Vulnerability details Impact There is no limit to the number of shareholders. It is therefore possible to set a large number of shareholders such that sendFees will run out of gas when adding shares to each shareholder. This will cause denial of service to all NestedFactory...

A user allowed to mint AlToken can mint an unlimited amount of it

Handle 0x0x0x Vulnerability details Impact A user with permission of minting can mint unlimited tokens. Proof of Concept We assume the user is allowed to mint, so the user is whitelisted, not blacklisted and ceilingmsg.sender 0. Furthermore, we assume that the contract is not paused. So when a us...

Missing _token.approve() to curvePool in setZapConfig

Handle WatchPug Vulnerability details function setZapConfig uint256 idx, address sett, address token, address curvePool, address withdrawToken, int128 withdrawTokenIndex external onlyGovernance; requiresett != address0; requiretoken != address0; require withdrawToken == addressWBTC || withdrawTok...

setGuardian incorrectly set governance in SettToRenIbbtcZap.sol

Handle gzeon Vulnerability details Impact setGuardian incorrectly set governance in SettToRenIbbtcZap.sol Proof of Concept onlyGovernance; governance = guardian; Tools Used Recommended Mitigation Steps --- The text was updated successfully, but these errors were encountered: All reactions...

Excessive require makes the transaction fail unexpectedly

Handle WatchPug Vulnerability details The check for RENCRVVAULT.blockLock is only needed when if amounts1 0 || amounts2 0. However, in the current implementation, the check is done at the very first, making transactions unrelated to RENCRVVAULT fail unexpectedly if there is a prior transaction...

DOS in LinearVesting

Handle hack3r-0m Vulnerability details 1. vestFor allows anyone to vest on anyone's behalf. 2. vestFor allows to vest only once per user so if some user wants to vest 1000 tokens, then a malicious actor can vest 1 token on behalf of that user. it will stop user from vesting 1000 tokens. This can...

Users Can Reset Bond Depositor's Vesting Period

Handle leastwood Vulnerability details Impact The VaderBond.deposit function overwrites a depositors bond info on each call with the updated payout information. If any of the vesting is left unclaimed before a call to deposit is made, the vesting period is reset to terms.vestingTerm, resulting in...

setGuardian incorrectly set governance in IbbtcVaultZap.sol

Handle gzeon Vulnerability details Impact setGuardian incorrectly set governance in IbbtcVaultZap.sol Proof of Concept onlyGovernance; governance = guardian; Tools Used Recommended Mitigation Steps --- The text was updated successfully, but these errors were encountered: All reactions...

setReserve() can be front-run

Handle palina Vulnerability details Impact The reserve address variable in NestedFactory.sol remains equal to 0 before the setReserve function is called by an owner. This may lead to incorrect transfers of tokens or invalid comparison with e.g., the asset reserve nestedRecords.getAssetReservenftI...

No slippage control on deposit of IbbtcVaultZap.sol

Handle gzeon Vulnerability details Impact There is no slippage control on deposit of IbbtcVaultZap.sol, which expose user to sandwich attack. Proof of Concept Any deposit can be sandwiched, especially when the pool is not balanced. Tools Used Recommended Mitigation Steps Add a minOut in line with...

redeem may return less than minOut

Handle gzeon Vulnerability details Impact redeem may return less than minOut if wrong token is supplied with poolID=3 because there is no check against minOut in L230-L232. Proof of Concept 1. User mistakenly call redeemUSDC, 100, 3, 0, 100 2. The contract take 100 bBTC from the user, redeem it...

Incorrect Accrual Of sumNative and sumUSD In Producing Consultation Results

Handle leastwood Vulnerability details Vulnerability details Impact The TwapOracle.consult function iterates over all token pairs which belong to either VADER or USDV and then calculates the price of the respective asset by using both UniswapV2 and Chainlink price data. This helps to further...

setGaurdian is changing Governance instead of Gaurdian

Handle fatimanaz Vulnerability details Impact In file ibBTC VaultZap.sol , The function setGaurdian is setting the governance. First problem is , setGaurdian is supposed to set gaurdian not governance. gaurdian is a different contract which does not have access to many functions, if we are making...

mint() Input Variable minOut Does Not Pass Value to interface ICurveFi add_liquidity()

Handle Meta0xNull Vulnerability details Impact Dev Note in Zap.sol: @param minOut Minimum amount of ibbtc to mint. Use for capping slippage while adding liquidity to curve pool. User Input minOut in function mint: function mintIERC20 token, uint amount, uint poolId, uint idx, uint minOut In...

calcMint always return poolId=0 and idx=0

Handle gzeon Vulnerability details Impact calcMint in Zap.sol always return poolId=0 and idx=0, while the docstring specified it should return the most optimal route instead. This will lead to suboptimal zap. Proof of Concept Tools Used Recommended Mitigation Steps --- The text was updated...

setGaurdian is setting Governance instead of governance in SettToRenIbbtcZap.sol

Handle fatimanaz Vulnerability details Impact In file ibBTC VaultZap.sol , The function setGaurdian is setting the governance. First problem is , setGaurdian is supposed to set gaurdian not governance. gaurdian is a different contract which does not have access to many functions, if we are making...

setGuardian() Wrong implementation

Handle WatchPug Vulnerability details function setGuardianaddress guardian external onlyGovernance; governance = guardian; function setGuardianaddress guardian external onlyGovernance; governance = guardian; governance = guardian should be guardian = guardian. --- The text was updated successfull...

Access control : wrongly setting the new guardian address to governance address

Handle ksk2345 Vulnerability details Impact Impact is high; once a new guardian is set using 'setGuardian' function, the new guardian is in full control of the contract, due to this bug. The setGuardian function mistakenly sets the governance address instead of setting it to guardian. Proof of...

IbbtcVaultZap.sol#deposit() can be front run

Handle WatchPug Vulnerability details function deposituint2564 calldata amounts public whenNotPaused // ... Given that IbbtcVaultZap.soldeposit will add liquidity to the curve pool, and the amount out differs when the price of tokens in the pool changes. However, the current implementation provid...

Improper implementation of slippage check

Handle WatchPug Vulnerability details function redeemIERC20 token, uint amount, uint poolId, int128 idx, uint minOut external defend blockLocked whenNotPaused returnsuint out ibbtc.safeTransferFrommsg.sender, addressthis, amount; Pool memory pool = poolspoolId; if poolId = minOut, "Slippage Check...

setGuardian() Privilage Escalation Causing Governance Lose Control of The Contract

Handle Meta0xNull Vulnerability details Impact governance = guardian The Guardian will become Governance of the Contract which is Not Expected. Original Governance will lose control of this contract if they call setGuardian with Address/Key beyong their control. Proof of Concept Tools Used Manual...

TWAPOracle might register with wrong token order

Handle cmichel Vulnerability details The TWAPOracle.registerPair function takes in a factory and token0, token1. The function accepts a factory argument which means any Uniswap-like factory can be used. When using the actual Uniswap factory's IUniswapV2Factoryfactory.getPairtoken0, token1 call, i...

All user assets which are approved to VaderPoolV2 may be stolen

Handle TomFrench Vulnerability details Impact Total loss of funds which have been approved on VaderPoolV2 Proof of Concept VaderPoolV2 allows minting of fungible LP tokens with the mintFungible function Crucially this function allows a user supplied value for from which specifies where the...

VaderRouter._swap performs wrong swap

Handle cmichel Vulnerability details The 3-path hop in VaderRouter.swap is supposed to first swap foreign assets to native assets, and then the received native assets to different foreign assets again. The pool.swapnativeAmountIn, foreignAmountIn accepts the foreign amount as the second argument...

Lack of access control allow attacker to mintFungible() and mintSynth() with other user's wallet balance

Handle WatchPug Vulnerability details function mintFungible IERC20 foreignAsset, uint256 nativeDeposit, uint256 foreignDeposit, address from, address to external override nonReentrant returns uint256 liquidity IERC20Extended lp = wrapper.tokensforeignAsset; require lp != IERC20ExtendedZEROADDRESS...