CoreCollection's token transfer can be disabled

Lines of code Vulnerability details Impact When royaltyAsset is an ERC20 that doesn't allow zero amount transfers, the following griefing attack is possible, entirely disabling CoreCollection token transfer by precision degradation as both reward distribution and vault balance can be manipulated...

[WP-M10] Lack of access control allow anyone to withdrawInterest() for any lender

Lines of code Vulnerability details function withdrawInterestuint256 id, address lender external nonReentrant withdrawInterestid, lender; function withdrawInterestuint256 id, address lender internal address strategy = pooledCLConstantsid.borrowAssetStrategy; address borrowAsset =...

Ineffective Handling of FoT or Rebasing Tokens

Lines of code Vulnerability details Impact Certain ERC20 tokens may change user's balances over time positively or negatively or charge a fee when a transfer is called FoT tokens. The accounting of these tokens is not handled by RoyaltyVault.sol or Splitter.sol and may result in tokens being stuc...

Pool Credit Line May Not Able to Start When _borrowAsset is Non ERC20 Compliant Tokens

Lines of code Vulnerability details Impact IERC20borrowAsset.transferto, fee; If the USDT token is supported as borrowAsset, the unsafe version of .transferto, fee may revert as there is no return value in the USDT token contract’s transfer implementation but the IERC20 interface expects a return...

createProject can be frontrun

Lines of code Vulnerability details Impact This is dangerous in scam senario because the malicious user can frontrun and become the owner of the collection. As owner, one can withdraw paymentToken. note that collections.isForSale can be change by frontrunner Proof of Concept 1. Anyone can call...

Any User Can Register Them-self if signerAddress is Unset

Lines of code Vulnerability details Impact The twitter verifier contract will sign twitter accounts using some signerAddress. However, if for whatever reason this is unset or happens to be the zero address, then anyone could bypass the requiresigner == signerAddress, 'RS5'; check in registerSelf...

Incorrect value subtracted from sharesHeld for closed and liquidated pools in LenderPool._withdrawLiquidity

Lines of code Vulnerability details Impact sharesHeld is used to track the amount of yield shares held by LenderPool. However, in the withdrawLiquidity function, when both principal and interest are withdrawn, sharesHeld is only modified by the interest part. This results in incorrect bookkeeping...

Fee in start() can be avoided

Lines of code Vulnerability details Impact A fee is collected in start that does not get collected if the borrowLimit is reached in lend. Proof of Concept if a start gets called and the amount - fee minBorrowAmount then a fee gets collected before accept is called. if maxLent is met in lend then...

Denial of services in proxy context by setting immutable privileged addresses in constructor in upgradeable contracts

Lines of code Vulnerability details Impact Privileged immutable addresses in LenderPool such as POOLEDCREDITLINE, SAVINGSACCOUNT and VERIFICATION are set in the constructor in the logic contract. These values are run at the time of deployment and affect only the local storage of the logic contrac...

cooldown is set to 0 when the user sends all tokens to himself.

Lines of code Vulnerability details Impact In the beforeTokenTransfer function, cooldowns will be set to 0 when the user transfers all tokens to himself. Consider the following scenario Day 0: The user stakes 100 tokens and calls the cooldown function Day 10: the user wanted to unstake the tokens...

LenderPool: Principal withdrawable is incorrectly calculated if start() is invoked with non-zero start fee

Lines of code Vulnerability details Details & Impact The principalWithdrawable calculated will be more than expected if start is invoked with a non-zero start fee, because the borrow limit is reduced by the fee, resulting in totalSupplyid not being 1:1 with the borrow limit. function...

Use safeTransfer

Lines of code Vulnerability details Impact The return value of the transfer is not checked. You already imported SafeERC20 into this contract, so you can use it to resolve this issue. Proof of Concept Tools Used manual code review Recommended Mitigation Steps IERC20borrowAsset.safeTransferto, fee...

_withdrawLiquidity() is Not Consistent With start()

Lines of code Vulnerability details Impact withdrawLiquidity details a number of scenarios under which a user may withdraw their liquidity. The first two scenarios outline cases where the credit line has either been cancelled by the borrower or insufficient assets have been lent out to the pool b...

Multiple missing approve(spender, 0) before approve(spender, amount)

Lines of code Vulnerability details Impact There are a few instances of missing calls to the IERC20 interface's approve function. The allowance needs to be set to 0 by calling for a given address and spender IERC20address.approvespender, 0. Not only do tokens like USDT require the allowance to be...

PooledCreditLine: termination likely fails because _principleWithdrawable is treated as shares

Lines of code Vulnerability details Details & Impact principalWithdrawable is denominated in the borrowAsset, but subsequently treats it as the share amount to be withdrawn. // notBorrowed = borrowAsset amount that isn't borrowed // totalSupplyid = ERC1155 total supply of id // borrowedTokens =...

DoS: Attacker May Front-Run CoreFactory.createProject() Or CoreFactory.addCollection() With A collection.id Causing Future Transactions With The Same collection.id to Revert

Lines of code Vulnerability details Impact A collection.id may only be used once in CoreFactory.createCollection since the the contract is deployed using the create2 opcode with a repeated salt and contract bytecode will fail to deploy a contract. Furthermore, the modifier onlyAvailableCollection...

Deposited collateral can't be withdrawn when PooledCreditLineStatus is Expired

Lines of code Vulnerability details Impact A user who deposits on an PooledCreditLineStatus.EXPIRED will not be able to withdraw the collateral Proof of Concept 1. Alice uses depositCollateral with 100 USDC on an Expired credit line - Here's the code 2. Alice attempts to use withdrawCollateral th...

Result of transfer not checked

Lines of code Vulnerability details Impact A call to transfer is done without checking the results. For certain ERC20 tokens, if insufficient tokens are present, no revert occurs but a result of "false" is returned. So it's important and also a best practice to check this. Note that, in almost al...

Potentially depositing at unfavorable rate since anyone can deposit the entire lenderPool to a known strategy at a pre-fixed time

Lines of code Vulnerability details Impact An attacker could keep track of the totalSupply of each LenderPool to see if it is more than the minBorrowAmount. If so, at startTime, which is pre-announced, the attacker could call start, which will trigger SAVINGSACCOUNT.deposit of the entire pool...

[WP-H1] LenderPool.sol#terminate() Wrong value is used for the shares argument of SAVINGS_ACCOUNT.withdrawShares() can cause fund loss to all users

Lines of code Vulnerability details / @notice Function invoked when pooled credit line is terminated by admin @dev only pooledCreditLineContract can invoke @param id identifier for the pooled credit line @param to address to which all the borrow tokens are transfered / function terminateuint256 i...

approve and safeApprove Should Approve the Zero Amount First

Lines of code Vulnerability details Impact The PooledCreditLine.sol and LenderPool.sol contracts approve the strategy contract on the collateral or borrow assets before depositing funds. This allows the strategy contracts to transfer on behalf of these contracts, pulling assets out from the pool...

Gas costs will likely result in any fees sent to the Splitter being economically unviable to recover.

Lines of code Vulnerability details Impact Collection owners will likely lose money by claiming fees unless the fees from a single NFT sale outweighs the cost of claiming it not guaranteed. Proof of Concept Consider a new Collection with a RoyaltyVault and Splitter set and a nonzero mint fee. Whe...

[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-H9] LenderPool.sol#start() startFeeFraction can be used by a malicious/compromised owner to rug lenders

Lines of code Vulnerability details A configurable startFeeFraction with no upper bound can be claimed by the caller to a specified address. The fee is not based on the gas cost, but on the totalLent of the pool. We believe this startFee reward is unnecessary and it creates a potential rug vector...

[WP-M7] Wrong design/implementation of interest accrued to the borrowers can make the lenders to end up getting much fewer amount of interest than expected

Lines of code Vulnerability details function calculateInterest uint256 principal, uint256 borrowRate, uint256 timeElapsed internal pure returns uint256 return principal.mulborrowRate.multimeElapsed.divYEARINSECONDS.divSCALINGFACTOR; function updateStateOnPrincipalChangeuint256 id, uint256...

updateSignValidity() May Break registerSelf() Due to Lack of Input Validation

Lines of code Vulnerability details Impact requireblock.timestamp Recommended Mitigation Steps requiresignValidity != 0, "signValidity Can't Be Zero" --- The text was updated successfully, but these errors were encountered: All reactions...

DoS: Attacker May Front-Run createSplit() With A merkleRoot Causing Future Transactions With The Same merkleRoot to Revert

Lines of code Vulnerability details Impact A merkleRoot may only be used once in createSplit since it is used as salt to the deployment of a SplitProxy. The result is an attacker may front-run any createSplit transaction in the mem pool and create another createSplit transaction with a higher gas...

[WP-H3] Proxy admin of the upgradeable proxy contracts can steal _borrowAsset and collateralAsset from the contracts and users' wallet

Lines of code Vulnerability details Both LenderPool and PooledCreditLine are upgradeable contract that holds users' allowances, and in certain periods, LendingPool will be holding users' funds. Use of Upgradeable Proxy Contract Structure allows the logic of the contract to be arbitrarily changed...

Calling approve() without first calling approve(0) causes problems with non-standard tokens (e.g. USDT)

Lines of code Vulnerability details This is another instance of the same issue that was found in the last sublime contest. In that issue the judge upgraded the finding to a risk of Medium. Impact Calling approve without first calling approve0 will revert with some tokens, such as Tether USDT. Thi...

_withdrawLiquidity() is Not Consistent With start()

Lines of code Vulnerability details Impact withdrawLiquidity details a number of scenarios under which a user may withdraw their liquidity. The first two scenarios outline cases where the credit line has either been cancelled by the borrower or insufficient assets have been lent out to the pool b...

LenderPool.terminate mix calculation between shares and tokens, resulting in incorrect amount of shares to be withdrawn from savings account

Lines of code Vulnerability details Impact While calculating the amount of shares to withdraw in LenderPool.terminate, principalWithdrawable which is amount of tokens is added to totalInterestInShares, and then passed to SAVINGSACCOUNT.withdrawShares. If tokens : shares are not 1 : 1, which is th...

DoS: Attacker May Front-Run CoreFactory.createProject() With A _projectId Causing Future Transactions With The Same _projectId to Revert

Lines of code Vulnerability details Impact A projectId may only be used once in CoreFactory.createProject since the modifier onlyAvailableProject will revert if project.creator != 0. The result is an attacker may front-run any createProject transaction in the mem pool and create another...

Reentrancy

Lines of code Vulnerability details Impact Potential Reentrancy in staking/unstaking function Proof of Concept Reentrancy in HolyPaladinToken.stakeAndIncreaseLock contracts/HolyPaladinToken.sol346-365: External calls: - stakedAmount = stakemsg.sender,amount contracts/HolyPaladinToken.sol353...

Incorrect borrowLimit used in calculating principal to withdraw for non-activated pools that are terminated

Lines of code Vulnerability details Impact LenderPool does not check the status of CreditLine when admin calls terminate. Thus if a careless admin discovered some malicious borrower and accidentaly terminated the pool before it went active, excessive tokens will be withdrawn, resulting in stolen...

Owner Can Stop Bridge All Functionality Only With One Change

Lines of code Vulnerability details Impact Owner can stop all bridge functionality with the changing cBridge contract address with the zero. The centralized structure can cause to pause all operations. Proof of Concept Tools Used Code Review Recommended Mitigation Steps It is recommended to place...

Using payable.transfer functions in WithdrawFacet.sol and Libasset.sol is not usable for smart contract calls due to possible shortage of gas.

Lines of code Vulnerability details Impact Withdrawals and transferERC20 tokens are executed via transferERC20 and withdraw functions. Since these functions calls with a fixed amount of gas, it's not yet guaranteed to reach to the destination if the receiver is a smart contract. Proof of Concept...

DoS: User's May Claim _splitId Causing Other Transactions to Revert

Lines of code Vulnerability details Impact Users may set splitId to any value when calling SplitFactory.createSplit allowing the user to claim this ID. This is exploitable to front-running in a denial of service attack where an attacker may call see a transaction in the mempool and call...

Improper Token Balance Check on swap()

Lines of code Vulnerability details Improper Token Balance Check on swap Description The swap can be separated in 2 paths, swap native to ERC20, or swap ERC20 to native. The contract performs a fromAssetId balance check before calling swap, mean that the attacker could periodically check for ERC2...

Users can transfer tokens to themselves to set cooldowns to 0, and then increase the lock time of other users' tokens when transferring to other users

Lines of code Vulnerability details Impact In the beforeTokenTransfer function, cooldowns will be set to 0 when the user transfers all tokens to himself. function beforeTokenTransfer address from, address to, uint256 amount internal virtual override iffrom != address0 //check must be skipped on...

Borrower funds can get stuck

Lines of code Vulnerability details Impact If request status is expired then full borrower collateral amount will get stuck as withdrawableCollateral will always be 0 even though borrower has repaid 90% of the amount Proof of Concept 1. User A deposit collateral against his request using...

use of transfer() instead of call() to send eth

Lines of code Vulnerability details Impact Use of transfer might render ETH impossible to withdraw becuase after istanbul hardfork , there is increases in the gas cost of the SLOAD operation and therefore breaks some existing smart contracts.Those contracts will break because their fallback...

if msg.value > amount , then extra eth is not transfered back to user

Lines of code Vulnerability details Impact if msg.value amount , there is no mechanism to send extra eth back due to which user will lose extra Proof of Concept function startBridgeTokensViaCBridgeLiFiData memory lifiData, CBridgeData calldata cBridgeData public payable if cBridgeData.token !=...

admin can rug

Lines of code Vulnerability details admin can steal all user funds --- The text was updated successfully, but these errors were encountered: All reactions...

[WP-H10] GenericSwapFacet.sol#swapTokensGeneric() duplicated .call{ value: msg.value } makes it possible for the attacker to steal native tokens (ETH) from the contract

Lines of code Vulnerability details function swapTokensGenericLiFiData memory lifiData, LibSwap.SwapData calldata swapData public payable uint256 receivingAssetIdBalance = LibAsset.getOwnBalancelifiData.receivingAssetId; // Swap executeSwapslifiData, swapData; uint256 postSwapBalance =...

ALMOST DEPRECATED TRANSFER() IS USED TO WITHDRAW ETHER

Lines of code Vulnerability details Impact transfer function can cause withdrawal to fail Proof of Concept function withdraw address assetAddress, address to, uint256 amount public LibDiamond.enforceIsContractOwner; address sendTo = to == address0 ? msg.sender : to; uint256 assetBalance; if...

Incorrect implementation of the batchRemoveDex function in DexManagerFacet

Lines of code Vulnerability details Impact The batchRemoveDex function does not work as expected. It should remove all the given DEX addresses from the dexWhitelist. However, it only removes the first successfully found DEX address and then stops removing the rest. The functionality is broken, an...

twitterId is unrelaible source of link

Lines of code Vulnerability details Impact Registration link cannot be relied upon. After the user is registered, User can easily change there twitterId to something else. The old id can later be acquired by someone else, thus making registration link to incorrect person Proof of Concept 1. User ...

Fixed Amount of Gas Sent in Call May Be Insufficient

Lines of code Vulnerability details Impact The function attemptETHTransfer makes a call with a fixed amount of gas, 30,000. If the receiver is a contract this may be insufficient to process the receive function. As a result the user would be unable to receive funds from this function. Proof of...

Existing credit line can be overwritten

Lines of code Vulnerability details Impact If pooledCLConstants1 already exists and Admin calls create function with id as 1 then pooledCLConstants1 will be overwritten with new value Proof of Concept 1. User A calls the create function to prepare a new credit line with id 1 2. This credit line i...

onlyOwner modifier missing

Lines of code Vulnerability details Impact Function is missing onlyOwner modifier meaning any user could register without need to be an owner Proof of Concept registerSelf function is missing the onlyOwner modifier. Recommended Mitigation Steps Change definition to function registerSelf...