User will lose value on swap-and-bridge / multi-swap

Lines of code Vulnerability details Impact In a swap-and-bridge or multi-swap, user have to supply exact amount of input token for each step. Any positive slippage will be captured by the contract and any negative slippage will cause the swap to revert. This is a sub-optimial behavior as it will...

Swap Functions Do Not Verify Final Token Matches The Swapped Token

Lines of code Vulnerability details Impact When calling Swapper.executeSwaps there are no checks to ensure the received token matches the final swapped token. If these are different it may result in user funds being locked in the contract. This issue is present in each of the following functions:...

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

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

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

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

[WP-M11] Wrong implementation of DexManagerFacet.sol#batchRemoveDex() will remove only the first item

Lines of code Vulnerability details function batchRemoveDexaddress calldata dexs external LibDiamond.enforceIsContractOwner; for uint256 i; i dexs.length; i++ if s.dexWhitelistdexsi == false continue; s.dexWhitelistdexsi = false; for uint256 j; j s.dexs.length; j++ if s.dexsj == dexsi removeDexj;...

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

dangerous payable function

Lines of code Vulnerability details A user might accidently send ether if sendingAssetId is not ether. Recommendation add in line 54: requiremsg.value == 0; --- The text was updated successfully, but these errors were encountered: All reactions...

[WP-H7] Infinite approval to an arbitrary address can be used to steal all the funds from the contract

Lines of code Vulnerability details function startBridgeAnyswapData memory anyswapData internal // Check chain id requireblock.chainid != anyswapData.toChainId, "Cannot bridge to the same network."; address underlyingToken = IAnyswapTokenanyswapData.token.underlying; if underlyingToken ==...

GenericSwapFacet misuses _lifiData

Lines of code Vulnerability details Impact https://github.com/code-423n4/2022-03-lifinance/blob/main/docs/GenericSwapFacet.md stated that lifiData is strictly for analytics purposes. But lifiData is used to set receivingAsset. Proof of Concept In GenericSwapFacet.swapTokensGeneric,...

[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-M9] sendNative with CBridge will always revert

Lines of code Vulnerability details function startBridgeCBridgeData memory cBridgeData internal Storage storage s = getStorage; address bridge = bridge; // Do CBridge stuff requires.cBridgeChainId != cBridgeData.dstChainId, "Cannot bridge to the same network."; if...

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

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

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

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

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

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

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

Borrower can stop lenders from withdrawing interest payments

Lines of code line Vulnerability details Impact A borrower can prevent lenders from being able to withdraw any interest payments. For example, a malicious borrower could create a credit line proposal with a long duration and a promise of regular interest payments to entice lenders to deposit into...

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

Constructor cannot be used in upgradeable contracts

Lines of code Vulnerability details Details As per OpenZeppelin’s documentation, “Due to a requirement of the proxy-based upgradeability system, no constructors can be used in upgradeable contracts.” reason being “the code within a logic contract’s constructor will never be executed in the contex...

Reliance on lifiData.receivingAssetId can cause loss of funds

Lines of code Vulnerability details Impact In the swapTokensGeneric function, an arbitrary number of swaps can be performed from and to various tokens. However, the final balance that is sent to the user relies on lifiData.receivingAssetId which has no use in the swapping functionality. LifiData ...

Must approve 0 first

Lines of code Vulnerability details Impact Some tokens like USDT do not work when changing the allowance from an existing non-zero allowance value.They must first be approved by zero and then the actual allowance must be approved. Proof of Concept Tools Used None Recommended Mitigation Steps Use...

Missing input validation could lead to loss of fund

Lines of code Vulnerability details Impact A faulty input in GenericSwapFacet.swapTokensGeneric could cause funds to get stuck in the contract. In addition, tokens left in the LiFi contract can be retrieved by anyone see issue: ERC20 withdrawals can be frontrun, leading to loss of fund. Proof of...

Incorrect number of seconds in ONE_YEAR variable

Lines of code Vulnerability details Impact In HolyPaladinToken.sol the ONEYEAR variable claims that there are 31557600 seconds in a year when this is incorrect. The ONEYEAR variable is used in the getCurrentVotes function as well as the getPastVotes function so it is vital that the correct time i...

Missing require can lead to funds lost

Lines of code Vulnerability details Failed transfer with low level call won't revert Impact A missing require may cause user to lose funds if a corner case issue filed separately named Failed transfer with low level call won't revert. This may result in user funds lost because funds were...

A swap with a token amount received of zero does not fail

Lines of code Vulnerability details Impact A token swap via NXTPFacet::swapAndCompleteBridgeTokensViaNXTP in which the receiving amount of tokens is zero does not fail. Proof of Concept The function NXTPFacet::swapAndCompleteBridgeTokensViaNXTP does not require that the token balance after the sw...

Lack of checks between _swapData and _lifiData could lead to loss of funds and reputation risk.

Lines of code LibSwap.swap swapTokensGeneric Vulnerability details Impact Users could input incongruent values for lifiData and swapData leading to a swap no being processed correctly and users not getting any of the expected lifiData.receivingAssetId. It can also damage reputation because LiFi...

Changing a token buy price during the sale by front-running buyers by forcing them to purchase at an unfair token price

This issue has been created to subdivide a multi-part submission to a single, medium severity finding. See issue 50 and in particular, judge @0xleastwood's comment here. --- The text was updated successfully, but these errors were encountered: All reactions...

Funds are transferred to saleRecipient before settlement

This issue has been created to subdivide a multi-part submission to a single, medium severity finding. See issue 50 and in particular, judge @0xleastwood's comment here. --- The text was updated successfully, but these errors were encountered: All reactions...

Low-level transfer via call() can fail silently

Lines of code Vulnerability details Impact In the call function in TimelockController.sol, a call is executed with the following code: function call bytes32 id, uint256 index, address target, uint256 value, bytes memory data private // solhint-disable-next-line avoid-low-level-calls bool success,...

[WP-H6] Admin of the upgradeable proxy contract of Controller.sol can rug users

Lines of code Vulnerability details Use of Upgradeable Proxy Contract 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 can be performed by th...

Use of deprecated Chainlink function latestAnswer

Lines of code Vulnerability details function getCurrentPriceaddress asset external view overrideProviderOracleManager, IProviderOracleManager returns uint256 address assetOracle = getAssetOracleasset; IEACAggregatorProxy aggregator = IEACAggregatorProxyassetOracle; int256 answer =...

Spreads can be minted with a deactivated oracle

Lines of code Vulnerability details Impact When deactivateOracle is called for an oracle in OracleRegistry it is still available for option spreads minting. This way a user can continue to mint new options within spreads that rely on an oracle that was deactivated. As economic output of spreads i...

Incorrect strike price displayed in name/symbol of qToken

Lines of code Vulnerability details Impact slice in options/QTokenStringUtils.sol cut a string into stringstart:end However, while fetching bytes, it uses bytessstart+1 instead of bytessstart+i. This causes the return string to be composed of sstartend-start. The result of this function is then...

[WP-H4] Initializer of QuantConfig.sol can rug users

Lines of code Vulnerability details function initializeaddress payable timelockController public override initializer require timelockController != address0, "QuantConfig: invalid TimelockController address" ; AccessControlinit; Ownableinitunchained; setupRoleDEFAULTADMINROLE, msgSender;...

[WP-H0] Wrong implementation of EIP712MetaTransaction

Lines of code Vulnerability details 1. EIP712MetaTransaction is a utils contract that intended to be inherited by concrete actual contracts, therefore. it's initializer function should not use the initializer modifier, instead, it should use onlyInitializing modifier. See the implementation of...

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

[WP-H2] EIP712MetaTransaction.executeMetaTransaction() failed txs are open to replay attacks

Lines of code Vulnerability details Any transactions that fail based on some conditions that may change in the future are not safe to be executed again later e.g. transactions that are based on others actions, or time-dependent etc. In the current implementation, once the low-level call is failed...

[WP-H7] CollateralToken.sol's COLLATERAL_BURNER_ROLE and COLLATERAL_MINTER_ROLE can rug users

Lines of code Vulnerability details In the current design/implementation, CollateralToken is minted and burned by COLLATERALMINTERROLE and COLLATERALBURNERROLE. Such roles can be granted by the owner of QuantConfig, which is the deployer of the QuantConfig. Once the attacker managed to acquire on...

QTokens with the same symbol will lead to mistakes

Lines of code Vulnerability details The README.md states: Bob can then trade the QToken with Alice for a premium. The method for doing that is beyond the scope of the protocol but can be done via any smart contract trading platform e.g. 0x. It is therefore important that tokens be easily...

admin can rug by minting collateral tokens

Lines of code Vulnerability details The admin can set himself COLLATERALMINTERROLE, then mint unlimited collateral tokens, allowing them to drain the protocol. --- The text was updated successfully, but these errors were encountered: All reactions...

Mint spread collateral-less and conjuring collateral claims out of thin air with implicit arithmetic rounding and flawed int to uint conversion

Lines of code Vulnerability details Impact This report presents 2 different incorrect behaviour that can affect the correctness of math calculations 1. Unattended Implicit rounding in QuantMath.sol div and mul 2. Inappropriate method of casting integer to unsigned integer in SignedConverter.sol...

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

ConfigTimeLockController will put QuantConfig in a stalemate(rendering it unusable)

Lines of code Vulnerability details The QuantConfig contract has these important setters, setProtocolAddress, setProtocolUint256, setProtocolBoolean and setProtocolRole. This contract is subjected to a timelock before all such processes above are executed. But, the issue arises in the fact that i...

Usage of deprecated Chainlink functions

Lines of code Vulnerability details Impact The Chainlink functions latestAnswer and getAnswer are deprecated. Instead, use the latestRoundData and getRoundData functions. Proof of Concept Go to and search for latestAnswer or getAnswer. You'll find the deprecation notice. Tools Used none Recommend...

Chainlink pricer is using a deprecated API

Lines of code Vulnerability details Impact According to Chainlink's documentation, the latestAnswer function is deprecated. This function might suddenly stop working if Chainlink stop supporting deprecated APIs. And the old API can return stale data. Proof of Concept Tools Used None Recommended...

COLLATERAL_MINTER_ROLE can be granted by the deployer of QuantConfig and mint arbitrary amount of tokens

Lines of code Vulnerability details Impact function mintCollateralToken address recipient, uint256 collateralTokenId, uint256 amount external override require quantConfig.hasRole quantConfig.quantRoles"COLLATERALMINTERROLE", msg.sender , "CollateralToken: Only a collateral minter can mint...