Upgraded Q -> M from 207 [1656344422031]

Judge has assessed an item in Issue 207 as Medium risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

No support for fee on transfer tokens

Lines of code Vulnerability details Impact stake will revert for tokens that charge a fee on transfer. Proof of Concept Note: POC below assumes tokePoolContract.depositamount transfers part of Staking.sol balance to tokePoolContract. stake uses the amount as a reference for depositToTokemak and...

removeAddress doesn't decrease the contracts.length

Lines of code Vulnerability details Impact The contracts length will always increase because the removeAddress function just deleting the value inside the array and never decrease the length by calling pop method. This can lead to Dos when calling functions that doing loop on contracts storage:...

Staking.setCurvePool() doesn't approve allowance when changes CURVE_POOL.

Lines of code Vulnerability details Impact Staking.setCurvePool doesn't approve allowance when changes CURVEPOOL. It will affect when users exchange asset through CURVEPOOL. Proof of Concept When initialize the contract, Staking contract approves CURVEPOOL here. But when admin updates CURVEPOOL...

Uninitialized local variables

Lines of code Vulnerability details Impact Configuration -Check: uninitialized-local -Severity: Medium -Confidence: Medium -Description:Lender.approveaddress,address.i Lender.sol114 is a local variable never initialized.A smart contract with uninitialized local variables may lead to intentional...

Rug vector for admin of Lender.sol who can max approve all principal tokens for any market without waiting the 3 days to themselves

Lines of code Vulnerability details Impact The admin of Lender.sol is authorized to call the function approve which will grant any address max approval over the principal tokens of a particular market. I assume that this functionality is used to approve the respective redeemer contracts used in...

Critical address change

Lines of code Vulnerability details Impact Changing critical addresses in contracts should be a two-step process. in order to prevent changing admin by mistake Proof of Concept check these links: OpenZeppelin/openzeppelin-contracts1488 OpenZeppelin/openzeppelin-contracts2369 Tools Used Manual...

Yieldy._storeRebase() saves and emits wrong values.

Lines of code Vulnerability details Impact Yieldy.storeRebase saves and emits wrong values. I don't think the asset will be lost directly because of this but the rebase storage will have wrong values and it might affect the system later. Proof of Concept The previousCirculating must be a previous...

Removal of liquidity from the reserve can be griefed

Lines of code Vulnerability details Impact Users may be unable to withdraw/remove their liquidity from the LiquidityReserve if a user decides to grief the contract. Proof of Concept This is the only function in this contract that is able to unstake funds, so that they can be withdrawn/removed:...

_getTwav is not correct. It is not considering all last 4 blocks.

Lines of code Vulnerability details Impact This will result in invalid deadline to decide the buyout end. Proof of Concept /// @notice returns the TWAV of the last 4 blocks /// @return twav TWAV of the last 4 blocks function getTwav internal view returnsuint256 twav if...

NibblVault buyout duration longer than update timelock

Lines of code Vulnerability details Impact User can buy out NFT by initiating the process through initiateBuyout, then he has to wait BUYOUTDURATION which is 5 days and if the buyout will not get rejected he can claim the NFT. During that period bidder cannot cancel the process. The issue is that...

Buy logic is not correct

Lines of code Vulnerability details Impact This will affect the curves from where the buy need to happen. This will lead to erroneous result. Proof of Concept Tools Used Manual calculation and code review Recommended Mitigation Steps Update the Buy logic by referring the sell logic. --- The text...

Malicious Basket Could Be Used To Rug Fractionalized Token Holders

Lines of code Vulnerability details Proof-of-Concept The NibblVaultFactory.createVault accepts any NFT address or any Basket address. Therefore, an attacker could pass a malicious basket address during vault creation. function createVault address assetAddress, address curator, string memory name,...

sell() and buy() should not have whenNotPaused modifier

Lines of code Vulnerability details Impact Impacts are two fold: 1. admin should not be allowed to stop users from selling/cashing out their assets under any circumstance. 2. Due to poor timing or malicious admin activity, it could be the case that buy is put on pause right after initiateBuyout i...

Upgraded Q -> H from 285 [1655952312863]

Judge has assessed an item in Issue 285 as High risk. The relevant finding follows: 1. Excess ether sent to FixedPricePassThruGate is lost low passThruGate redirects to a beneficiary only gate.ethCost, requiring that msg.value = gate.ethCost. As there are no other ways to access native tokens hel...

Attacker can frontrun calling _mint_to_Accountant() which mints the entire supply to the caller, and sets them as _accountant and admin

Lines of code Vulnerability details Impact By leaving minttoAccountant with no access control when accountant = address0 it allows an attacker to call the function, mint the entire supply to themselves, and gain the accountant and admin roles. Additionally, the parameter "address...

repayAmountFinal is not considered while calculating actualRepayAmount while calling doTransferIn(payer, repayAmount)

Lines of code Vulnerability details Impact repayAmountFinal is not used inside repayBorrowFresh The computation for actualRepayAmount doesnot account for repayAmountFinal in L111. Proof of Concept function repayBorrowFreshaddress payer, address borrower, uint repayAmount internal override...

CNote: Attacker can frontrun the calling of _setAccountantContract and gain admin priviledges and insert a malicious accountant contract

Lines of code Vulnerability details Impact The function does not have access control before the accountant address is set, allowing anyone to call the function, gain admin privileges, and set the accountant address. Proof of Concept CNote.solL17 Recommended Mitigation Steps Include access control...

Stack too deep error within BaseV1-periphery.sol

Lines of code BaseV1-periphery.sol lines 323-341 Vulnerability details Impact Stack too deep error when using remove liquidity function, fixing issue allows for avoiding using optimizations thus increases the periphery contract's longevity/scalability. Proof of Concept function...

BridgeFacet's _executePortalTransfer ignores underlying token amount withdrawn from Aave pool

Lines of code Vulnerability details executePortalTransfer can introduce underlying token deficit by accounting for full underlying amount received from Aave unconditionally on what was actually withdrawn from Aave pool. Actual amount withdrawn is returned by IAavePools.aavePool.withdraw, but...

Upgraded Q -> M from 36 [1655654413719]

Judge has assessed an item in Issue 36 as Medium risk. The relevant finding follows: Allows owner to mint more tokens than planned There are two ways for the owner of the contract to mint tokens for themselves unrelated to the public sale: through the teamSummon function or by setting the minter ...

Wrong implementation of withdrawAdminFees() can cause the adminFees to be charged multiple times and therefore cause users' fund loss

Lines of code Vulnerability details function withdrawAdminFeesSwap storage self, address to internal IERC20 memory pooledTokens = self.pooledTokens; for uint256 i = 0; i pooledTokens.length; i++ IERC20 token = pooledTokensi; uint256 balance = self.adminFeesi; if balance != 0 token.safeTransferto,...

Did Not Approve To Zero First Causing Certain Token Transfer To Fail

Lines of code Vulnerability details Proof-of-Concept Some tokens like USDT do not work when changing the allowance from an existing non-zero allowance value. For example Tether USDT's approve function will revert if the current approval is not zero, to protect against front-running changes of...

Upgraded Q -> M from 225 [1655654402923]

Judge has assessed an item in Issue 225 as Medium risk. The relevant finding follows: C4-011 : Centralization Risk On The teamSummon Function - LOW Impact - LOW With the teamSummon function, owner can mint unlimited warriors. This poses a security risk. The max/min limit should be implemented at...

Funds(ETH) permanent lock on InfinityStaker.sol

Lines of code Vulnerability details Impact The rescueETH function does not work as expected and if the contract receives ETH, it gets stuck in the contract Proof of Concept If an addresswallet or contract send ETH to the InfinityStaker.sol contract, the owneradmin can't rescue the ETH because the...

_transferNFTs() functions doesn't reverts if the transfer item doesn't supports interface for both ERC721 and ERC1155

Lines of code Vulnerability details Impact The transferNFTs function use ERC165 to check if the itemnft supports ERC721 interface or ERC1155 interface and execute transfer accordingly. But if it doesn't supports either, it just exits the functionno revert. Proof of Concept in...

nonReentrant modifier is not added to all functions that generate state changes, there is a possibility of re-entry.

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. I noticed that the nonReentrant modifier only adds some of the functions that generate state changes to the InfinityExchange.sol contract, which provides the feasibility of reentrancy between multiple...

Malicious Relayer Could Cause A Router To Provide More Liquidity Than It Should

Lines of code Vulnerability details Proof-of-Concept Assume this is a fast-transfer path and the sequencer has a good reason e.g. some sophisticated liquidity load balancing algorithm to assign 3 routers to provide liquidity for a transfer of 90 DAI Therefore, each of them will provide 30 DAI...

Taker can provide several instances of the cheapest ERC-1155 item in a multi item bundle

Lines of code Vulnerability details Maker bid for a bundle of ERC-1155 items can be tricked into successful execution by providing several instances of the cheapest item instead of the required bundle. This way a malicious taker can receive full maker's price, providing several instances of the...

Missing slippage protection for autocompounding auraBAL rewards into AURA

Lines of code Vulnerability details Impact Autocompounding auraBAL rewards into AURA requires multiple swaps auraBAL - BAL/ETH BPT - WETH - AURA within MyStrategy.harvest. The swaps are at risk of being front-run / sandwiched, resulting in a loss of funds. Since MEV is very prominent I think the...

prepareWithdrawAll should not be external

Lines of code Vulnerability details Impact Despite commented as Internal Core Implementations, prepareWithdrawAll is external. There also lack guarantee that prepareWithdrawAll is called before withdrawAll. Proof of Concept /// ===== Internal Core Implementations ===== function prepareWithdrawAll...

_harvest has no slippage protection when swapping auraBAL for AURA

Lines of code Vulnerability details Impact Single swaps of harvest contains no slippage or deadline, which makes it vulnerable to sandwich attacks, MEV exploits and may lead to significant loss of yield. Proof of Concept When using BALANCERVAULT.swap here and here, there is no slippage protection...

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

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

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

Users Might Not Be Able To Purchase Or Redeem SetToken

Lines of code Vulnerability details Proof-of-Concept Whenever a setToken is issued or redeemed, the moduleIssueHook and moduleRedeemHook will be triggered. These two hooks will in turn call the redeemMaturedPositions function to ensure that no matured fCash positions remain in the Set by redeemin...

PortcalFacet.repayAavePortal() can trigger an underflow of routerBalances

Lines of code Vulnerability details Impact The caller of repayAavePortal can trigger an underflow to arbitrarily increase the caller's balance through an underflow. Proof of Concept // Relevant code sections: // PortalFacet.sol function repayAavePortal address local, uint256 backingAmount, uint25...

wfCashERC4626 maxWithdraw, previewWithdraw, previewRedeem, convertToAssets, convertToShares doesn't conform to EIP4626

Lines of code Vulnerability details EIP4626 states that maxWithdraw, convertToAssets, convertToShares, previewRedeem and previewWithdraw must not revert unless due to large input, or due to a reason that will make deposit/redeem revert. However, wfCash4626's implementation of those ends up callin...

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

Upgraded Q -> M from 307 [1655245513660]

Judge has assessed an item in Issue 307 as Medium risk. The relevant finding follows: Low feeRate can be modified for existing vaults feeRate is a parameter that controls the fee applied on exercise. It can be set by the function: function setFeeuint256 feeRate external onlyOwner feeRate = feeRat...

Withdraw function does not conform to EIP4626

Lines of code Vulnerability details Impact The withdraw of wfCashERC4626 is not 4626 compatible. wfCashERC4626.solL186-L191 According to EIP4626 Burns shares from owner and sends exactly assets of underlying tokens to receiver. The withdraw function of ERC4626 should send the exact same amount of...

Re-entrancy in wfCashERC4626.withdraw() can lead to more gains in assets

Lines of code Vulnerability details This is a corrected version of the previous submission and typo mistakes corrected. Impact The withdraw function in wfCashERC4626.sol can be re-entered at the point of redeemInternal. Assume asset tokens are sent to receiver after shares are burnt, and user...

Re-entrancy in wfCashERC4626.withdraw() can lead to more gains in assets

Lines of code Vulnerability details Impact The withdraw function in wfCashERC4626.sol can be re-entered at the point of redeemInternal. Assume asset tokens are sent to receiver after shares are burnt, and user re-enters withdraw after redeemInternal is completed., P.S: there's a separate issue on...

Loss of underlying tokens due to ERC4626 non-compliance in redeem function in wfCashERC4626.sol

Lines of code Vulnerability details Impact Similar to the report I sent earlier on the issue of The withdraw function in wfCashERC4626.sol, the redeem function is missing the code that transfers the underlying tokens to the receiver. According to the EIP-4626 standard, redeem function Burns share...

Upgraded Q -> M from 294 [1654474576938]

Judge has assessed an item in Issue 294 as Medium risk. The relevant finding follows: Owner can frontrun exercise to increase fees A malicious owner account can observe and frontrun calls to exercise and extract 100% of the strike price as a protocol fee. Scenario: A malicious owner observes a ca...

Usage of deprecated transfer to send ETH

Lines of code Vulnerability details Impact Usage of deprecated transfer Swap can revert. Proof of Concept The original transfer used to send eth uses a fixed stipend 2300 gas. This was used to prevent reentrancy. However this limit your protocol to interact with others contracts that need more th...

More than 1 zero address token in burnToTarget may lead to draining of FeeBurner.sol

Lines of code FeeBurner.solL43-L88 Vulnerability details Impact The burnToTarget function in FeeBurner.sol allows any array of tokens to be used. If the tokens array contains more than 1 zero address, the swapAll function for the swapperRouter will be called more than once with the same msg.value...

StakerVault.unstake(), StakerVault.unstakeFor() would revert with a uint underflow error of StakerVault.strategiesTotalStaked, StakerVault._poolTotalStaked.

Lines of code Vulnerability details Impact StakerVault.unstake, StakerVault.unstakeFor would revert with a uint underflow error of StakerVault.strategiesTotalStaked, StakerVault.poolTotalStaked. Proof of Concept Currently it saves totalStaked for strategies and non-strategies separately. uint...

Attacker can get drain ETH for targetLpToken_

Lines of code Vulnerability details Impact Attacker can drain all ETH from FeeBurner.sol. Technically msg.value gets sent to swapperRouter, but since this contract is out of scope FeeBurner.sol will be treated as the victim Proof of Concept FeeBurner.solL56-L65 ... for uint256 i; i tokens.length;...