Non unique token ID might lead to collusion

Lines of code Vulnerability details Impact Token IDs are defined as concatenation of points, total supply + 1. The total supply can decrease when split process in progress. This means that the contract might try to mint a token with an ID which already exists. Proof of Concept 1. Navigate to the...

Use safeTransfer instead of transferFrom

Lines of code Vulnerability details Impact In the following line, transferFrom is used. However, this function transfers funds from CoreCollection contract to msg.sender. That's why transfersafeTransfer must be used. Like the comment describes, if a royalty vault isn't set, tokens are kept within...

One co-creator with a small share can get 100% of the funds in the splitter

Lines of code Vulnerability details Impact One co-creator with a small share can get 100% of the funds by calling the incrementWindow function from an attacker contract that mimics RoyaltyVault. He can then create one or multiple fake windows and claim them to get the full balance of the splitter...

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

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

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

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

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

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

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

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

[WP-H6] Swapper can be used to steal all the funds 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 =...

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

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

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

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

Should prevent users from sending more native tokens in the startBridgeTokensViaCBridge function

Lines of code Vulnerability details Impact When a user bridges a native token via the startBridgeTokensViaCBridge function of CBridgeFacet, the contract checks whether msg.value = cBridgeData.amount holds. In other words, if a user accidentally sends more native tokens than he has to, the contrac...

All swapping functions lack checks for returned tokens

Lines of code Vulnerability details Impact Every function that stems from the GenericSwapFacet lacks checks to ensure that some tokens have been returned via the swaps. In LibSwap.sol in the swap function, the swap call is sent to the target DEX. A return of success is required, otherwise the...

[WP-M3] OperateProxy.callFunction() should check if the callee is a contract

Lines of code Vulnerability details /// @notice Allows a sender/signer to make external calls to any other contract. /// @dev A separate OperateProxy contract is used to make the external calls so /// that the Controller, which holds funds and has special privileges in the Quant /// Protocol, is...

Arbitrary code can be run with Controller as msg.sender

Lines of code Vulnerability details Impact A malicious user can call Controller's operate with ActionType.QTokenPermit, providing a precooked contract address as qToken, that will be called by Controller contract with IQTokenqToken.permit, which implementation can be arbitrary as long as IQToken...