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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Wrong design/implementation of addLiquidity() allows attacker to steal funds from the liquidity pool

Handle WatchPug Vulnerability details The current design/implementation of Vader pool allows users to addLiquidity using arbitrary amounts instead of a fixed ratio of amounts in comparison to Uni v2. We believe this design is flawed and it essentially allows anyone to manipulate the price of the...

Potential economic attack that exploits IL coverage

Handle WatchPug Vulnerability details The current implementation of Vader protocol provides impermanent loss coverage calculated as below: function calculateLoss uint256 originalVader, uint256 originalAsset, uint256 releasedVader, uint256 releasedAsset public pure returns uint256 loss // // TODO:...

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

Attacker can claim more IL by manipulating pool price then removeLiquidity

Handle gzeon Vulnerability details Impact Vader reimburse user IL immediately when user withdraw from the pool VaderRouterV2.sol:L227, an attacker can therefore manipulate the pool balance causing a high IL, remove liquidity and restore the pool balance such that he will receive a larger IL...