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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Users at UNSTAKE_PERIOD can assist other users in unstaking tokens.

Lines of code Vulnerability details Impact Consider the following scenario: Day 0: User A stakes 200 tokens and calls the cooldown function. At this time, user A's cooldown is Day 0. Day 15: User B stakes 100 tokens, but then wants to unstake tokens. So user A said that he could assist user B in...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Add a timelock to DiamondCutFacet

Lines of code Vulnerability details Impact To give more trust to users: functions that set key/critical variables should be put behind a timelock. Proof of Concept Tools Used Remix Recommended Mitigation Steps Add a timelock to setter functions of key/critical variables. --- The text was updated...

Any user can recover the funds left in the contract

Lines of code Vulnerability details Impact There is a WithdrawFacet such that only the owner/admin can recover the lost funds in the contract. However, any user can retrieve the funds by using the swapTokensGeneric function, which might be unexpected behavior. Proof of Concept 1. Suppose that 100...

Potential DoS

Lines of code Vulnerability details the balance of outputToken is checked to be exactly a specified value that is not declared in this specific function. Therefore, a malicious user can transfer to the contract address tiny amount of tokens and the user transactions will always revert. Potential...

Risk of centralization

Lines of code Vulnerability details Medium Risk Risk of centralization Impact Diamond owner has too many roles on setting the functions, initiating payable functions. If the Owner account is compromised, the assets may be drained in this trustless system. Proof of Concept Tools Used Static testin...

Interest accrued could be zero for small decimal tokens

Lines of code Vulnerability details Details & Impact Interest is calculated as principal.mulborrowRate.multimeElapsed.divYEARINSECONDS.divSCALINGFACTOR; It is possible for the calculated interest to be zero for principal tokens with small decimals, such as EURS 2 decimals. Accumulated interest ca...

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

Enforced Owner Can Extract Funds From The Contract

Lines of code Vulnerability details Impact During the code review, It has been observed that access control mechanisms are checked with the following line. LibDiamond.enforceIsContractOwner; The withdraw gaves abilitiy to contract owner extract all funds are sent to contract. This poses...

Using the native payable.transfer to send ETH in WithdrawFacet

Lines of code Vulnerability details Impact The withdraw function in WithdrawFacet uses the native transfer keyword to send ETH, which is considered unsafe because of the fixed gas budget, and its functionality could be broken in some circumstances: 1. The receiver consumes more than 2300 amounts ...

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

Owner can whitelist addresses for swaps and steal approved assets from users

Lines of code Vulnerability details Impact There is a common vulnerability with aggregator/bridge contracts where passing in arbitrary calldata can do unwanted actions such as steal tokens that were approved to that contract. While there is a whitelist system set up, there is no stopping a...

[WP-H8] Admin of the upgradeable proxy contract (the diamond contract) can rug users

Lines of code Vulnerability details Use of Upgradeable Proxy Contract Structure The Diamond Structure allows the logic of the contract to be arbitrarily changed. This allows the proxy admin to perform malicious actions e.g., taking funds from users' wallets up to the allowance limit. This action...

If contract holds balance of any ERC20 token, any user can take it

Lines of code Vulnerability details Impact If the LiFiDiamond contract ends up holding any ERC20 token, any user is able to perform a swap from the held asset to another asset, and the swap will use the contracts' funds as input instead of their own. The result is that a user can take all of the...

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

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

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

Incorrect implementation of the _startBridge function in CBridgeFacet

Lines of code Vulnerability details Impact The startBridge function in CBridgeFacet is to bridge the tokens to CBridge by calling the sendNative or send function on the bridge contract. However, when calling the sendNative function, no native token is sent to the bridge. The sendNative call alway...