LSP8Burnable extension incorrectly inherits LSP8IdentifiableDigitalAssetCore

Lines of code Vulnerability details Bug Description The LSP8Burnable contract inherits from LSP8IdentifiableDigitalAssetCore: LSP8Burnable.solL15 abstract contract LSP8Burnable is LSP8IdentifiableDigitalAssetCore However, LSP8 extensions are supposed to inherit LSP8IdentifiableDigitalAsset instea...

IF THE UNDERLYING ASSET IS A FEE ON TRANSFER TOKEN IT COULD BREAK THE INTERNAL ACCOUNTING OF THE VAULT

Lines of code Vulnerability details Impact The Vault.deposit function is used by the users to deposit assets to the vault and mint vault shares to the recipient address. The amount of assets are transferred to the Vault as follows: SafeERC20.safeTransferFrom asset, caller, addressthis,...

TwabLib::getTwabBetween can return innacurate balances if _startTime and _endTime aren't safely bounded

Lines of code Vulnerability details M-01 TwabLib::getTwabBetween can return innacurate balances if startTime and endTime aren't safely bounded Vulnerability details Here's the documentation of the get TwabLib::getTwabBetween function : File: twab-controller\src\libraries\TwabLib.sol 278: / 279:...

Malicious yield vault owners can manipulate the interaction between the vault and yield vault

Lines of code Vulnerability details Impact Malicious yield vault owners can steal depositor's assets. Proof of Concept yieldVault is the Address of the ERC4626 vault in which assets are deposited to generate yield. After tokens are deposited into the vault, the deposit function of yieldVault is...

No access control for mintYieldFee()

Lines of code Vulnerability details Impact Anyone can mint the yield fee using mintYieldFee. Proof of Concept mintYieldFee mints the shares for yield fee. function mintYieldFeeuint256 shares, address recipient external requireVaultCollateralized; if shares yieldFeeTotalSupply revert...

Inconsistencies When Using Inherited Functions

Lines of code https://github.com/code-423n4/2023-06-lukso/blob/9dbc96410b3052fc0fd9d423249d1fa42958cae8/contracts/LSP0ERC...

Incorrect Interface ID for LSP14

Lines of code Vulnerability details Impact Contracts that use typeILSP14Ownable2Step.interfaceId, such as when implementing IERC165.supportsInterface, will use a value different from the current standard. Proof of Concept The interface ID of LSP14 is stated to be 0x94be5999. However, the interfac...

Failed transfer with low level call could be overlooked

Lines of code Vulnerability details Impact Transfers may fail silently. Proof of Concept According to the Solidity docs: "The low-level functions call, delegatecall and staticcall return true as their first return value if the account called is non-existent, as part of the design of the EVM...

LSP8 and LSP9's ERC-165 interface ID differs from their specification

Lines of code Vulnerability details Bug Description According to LSP7's specification, the ERC-165 interface ID for LSP7 token contracts should be 0x5fcaac27: ERC165 interface id: 0x5fcaac27 However, INTERFACEIDLSP7 has a different value in the code: LSP7Constants.solL4-L5 // --- ERC165 interface...

Unrestricted Name and Symbol Modification in LSP7 and LSP8 Digital Assets

Lines of code Vulnerability details Impact The owner of a contract in LSP8IdentifiableDigitalAsset and LSP7DigitalAsset can arbitrarily change the name and symbol of a token after its deployment. This ability is due to the inheritance of the setData function from ERC725YCore.sol implemented in...

Incorrect amount of tokens is transferred before updating the accumulator and emitting the ContributePrizeTokens event

Lines of code Vulnerability details Impact An attacker can call the contributePrizeTokens function with a lower amount of tokens than claimed, misleading the contract to update the accumulator and emit the event based on the incorrect amount. As a result, the prize pool's internal state will be...

_winningRandomNumber vartiable used in calculating if caller is winner can be read via web3.getStorageAt() and user can predict if he wins or not before hand

Lines of code Vulnerability details Impact the variable winningRandomNumber used in isWinner to calc if caller has won the tier can be read, and the calculation can be predicted. Proof of Concept that a variable is marked internal doesnt mean it cannot be read Tools Used vs code Recommended...

The owner of a LSP0ERC725Account can become the owner again after renouncing ownership

Lines of code Vulnerability details Bug Description The renounceOwnership function allows the owner of a LSP0ERC725Account to renounce ownership through a two-step process. When renounceOwnership is first called, renounceOwnershipStartedAt is set to block.number to indicate that the process has...

Missing Input Validation

Lines of code Vulnerability details Impact The below situations do not have checks on their inputs: When bytes12inputDataKey == LSP6KEYADDRESSPERMISSIONSPERMISSIONSPREFIX: // AddressPermissions:Permissions: if bytes12inputDataKey == LSP6KEYADDRESSPERMISSIONSPERMISSIONSPREFIX // controller already...

isTimeSafe and isTimeRangeSafe not implemented in the functions getBalanceAt and getTwabBetween

Lines of code Vulnerability details Impact The Natspec of both getBalanceAt and getTwabBetween functions indicates that they should implement the functions isTimeSafe and isTimeRangeSafe respectively to ensure that the queried timestamps are safe, but both functions don't implement them which can...

Missing access control in mintYieldFee allowing everybody to mint the available YieldFee to himself

Lines of code Vulnerability details Impact Everybody can call the mintYieldFee function in the Vault, when there is yieldFeeTotalSupply available and mint shares to himself for free, which latter results in stealing funds form the Vault. if this is a desired behavior, which it shouldn't based on...

Permit does not revert for tokens that do not implement it.

Lines of code Vulnerability details Impact Callers should not rely on permit to revert for arbitrary tokens especially if permit is used as a security check. Tokens which do not revert on permit either do not implement it or have a non-reverting fallback function. Most notable among them is WETH...

Incorrect distribution of shares and liquidity as a result of total number of shares not equaling 100

Lines of code Vulnerability details Impact If the shares are not properly validated and do not add up to 100, there will be an imbalance in the distribution of funds resulting in loss of funds or locked funds that cannot be accessed or distributed correctly. Proof of Concept The constructor of th...

_getNextObservationIndex() Random use of timestamp to determine the currentTime can be manipulated bacause of dangerous strict equalities

Lines of code Vulnerability details Impact The use of strict equalities can be easily manipulated by an attacker. Miners may attempt to manipulate the timestamp. Proof of Concept File: TwabLib.sol Code Link: Code: if newestObservation.timestamp == currentTime File: TwabLib.sol Code Link: Code: if...

Vault.sponsor may take away the prize chance from the receiver.

Lines of code Vulnerability details Impact TwabController.delegateBalance is related to the probability to get the prize, and Vault.sponsor can make the others' delegateBalance to 0. A malicious user can send a small amount of assets to every depositor and be the only prize taker. Proof of Concep...

The output amount validation in Vault.liquidate() is not correct.

Lines of code Vulnerability details Impact The output amount validation is not correct in Vault.liquidate, so the method might accept invalid output amount and refuse valid output amount. Proof of Concept In Vault.liquidate, there is a validation about the output share amount should be less than ...

abi.encodePacked() collision due to dynamic types usage that could lead to

Lines of code Vulnerability details Impact The use of abi.encodePacked could lead to collision due to the dynamic types usage. Through abi.encodePacked, Solidity supports a non-standard packed mode where: Types shorter than 32 bytes are neither zero padded nor sign extended and Dynamic types are...

Vault funds can be stolen by a malicious Yield Vault.

Lines of code Vulnerability details Impact When a vault is initialized, it sets Max Token Approval for the Yield Vault which allows the Yield Vault to ALWAYS have access to the funds in the vault. Since vaults can be created by anyone as long as they provide an ERC-4626 compliant yield source, an...

initialize function can be front run

Lines of code Vulnerability details Impact Initialize function have the potential of front running by a malicious actor. An attacker can front-run the deployer and takeover the contract by setting itself as the owner in the Contract. Taking ownership will result in carrying out malicious acts tha...

Potential Reversion in Transfer due to LSP1 Interface Support Check

Lines of code Vulnerability details Impact The transfer function in LSP7DigitalAssetCore & LSP8DigitalAssetCore includes a mandatory hook, notifyTokenSender, which verifies if the sender supports INTERFACEIDLSP1. However, if a token owner who initially implemented LSP1 interface ceases to support...

No validation check for whether the msg.value is not less than the value parameter in the execute function

Lines of code Vulnerability details Impact Users can send msg.value lesser than the supplied value parameter leading to incorrect accounting. Proof of Concept There is no check for whether msg.value = value in the execute function of the LSP0ERC725AccountCore contract. LSP0ERC725Account is in sco...

All Ether sent to LSP0ERC725Account will be permanently locked

Lines of code Vulnerability details Impact All Ether sent to LSP0ERC725Account will be permanently locked because it inherits the receive function from the LSP0ERC725AccountCore contract but does not have a withdraw function. Proof of Concept All Ether sent to LSP0ERC725Account will be permanentl...

In LSP6SetDataModule.sol, Infinite loop in _verifyAllowedERC725YDataKeys()

Lines of code Vulnerability details Impact In verifyAllowedERC725YDataKeys there is a great possibility of an Infinite loop. This is because ++ii is an increment inside if condition. This can lead to excessive gas consumption, causing the Ethereum transaction to fail due to the gas limit Proof of...

Malicious user can steal other user's deposits from Vault.sol

Lines of code Vulnerability details Impact When the Vault.withdraw function is called, a maximum of typeuint96.max shares are being burnt subsequently: Vault.withdraw- Vault.withdraw- Vault.burn burns uint96shares, see Vault.sol line 1139. A malicious user can exploit this in the following way: 1...

Balance invariant between individual and total twabs can be broken

Lines of code Vulnerability details Impact An edge case in the TwabController.transferBalance can cause total balance for a vault account to decrease although it did not actually decrease. This will cause the sum of individual delegateBalances for a vault to be greater than the registered total f...

Attacker can steal vault funds through the deposit function.

Lines of code Vulnerability details Impact In the deposit function, a check is made to see if the amount of assets being deposited by the user is greater than the amount of assets the vault currently holds. The vault then transfers the difference between the assets being deposited and the vault’s...

Missing External Transfer Function In Vault

Lines of code Vulnerability details Impact Balances of TwabController for a vault can not be transferred. Proof of Concept The Vault implements an internal transfer function meant to be used to transfer balances within the TwabController: function transferaddress from, address to, uint256 shares...

Claim failure can result to loss of funds

Lines of code Vulnerability details Impact Caller will lose funds through gas fees when claim fails Proof of Concept The claimPrize function allows any caller to claim a prize by providing the necessary parameters: winner address of the winner, tier prize tier, prizeIndex prize index,...

Deposit transaction is prone to being front-run by bad actors.

Lines of code Vulnerability details Impact It is possible for an attacker to front-run a user's deposit transaction while transferring fewer amount of assets than the user and minting an equivalent amount of shares as the user could have. Proof of Concept The scenario described below is for the...

Attacker can force users to delegate to SPONSORSHIP_ADDRESS

Lines of code Vulnerability details Impact An attacker can change the delegatee of a user who deposited into the vault to the SPONSORSHIPADDRESS address by calling one of the functions sponsor or sponsorWithPermit and giving the address of the user as receiver. The impact of this issue is that th...

FIRST DEPOSITOR ATTACK IS PRESENT IN THE Vault.sol CONTRACT

Lines of code Vulnerability details Impact The Vault.deposit function is vulnerable to first depositor attack. It can be described as follows: A malicious early depositor can deposit with 1 wei of asset token as the first depositor of the Vault, and get 1 wei of shares. Then the first depositor c...

Unintended or Malicious Use of Prize Winners' Hooks

Lines of code Vulnerability details Impact The setHooks function in Vault.sol allows users to set arbitrary hooks, potentially enabling them to make external calls with unintended consequences. This vulnerability could lead to various unexpected behaviors, such as unauthorized side transactions...

Allowing delegate call with msg.value in executeBatch() is dangerous

Lines of code Vulnerability details Bug Description ERC725XCore's execute function allows four types of operations: 1. CALL for normal calls 2. DELEGATECALL 3. CREATE/CREATE2 for contract deployment 4. STATICCALL The executeBatch function simply calls execute in a loop to perform multiple calls i...

The exchange rate is decreasing in Vault

Lines of code Vulnerability details Impact The exchange rate of the vaults will be decreasing and this will block core functionalities. Proof of Concept The exchange rate of the Vault is calculated as follows: minwithdrawableAssets, totalSupplyToAssets assetUnit / totalSupplyAmount =...

Users might lose their balances when they set delegates

Lines of code Vulnerability details Impact Undelegated users will lose their balances if they set delegate to themselves. Proof of Concept A User can set a delegate and the delegated balance of the user will be accounted on the delegate's delegated balance in the TWAB controller. The internal...

propose function Can lead to unexpected behavior

Lines of code Vulnerability details Impact Problème in the propose function, specifically in the construction of the targets, values, signatures, and calldatas arrays. These arrays are used to define the transactions that will be executed within the proposal, So the problem arises from the use of...

Unused returns on the ds. _withdraw(), NounsDAOLogicV3._withdraw(), and ds.executeFork()

Lines of code Vulnerability details Impact The return value of the external calls are not stores in a local variable or a state variable Proof of Concept The return values do not store the results in a store local or state variable. As a result, the computation have no effect. Code:...

Malicious user may force gas limit error on created proposals

Lines of code Vulnerability details Impact User can create a proposal with arbitrary long array of targets. Function cancel iterates over those targets. Since this array may be long - the risk of out-of-gas error may occur - making the contract to be unable to cancel the proposal as cancel would...

Functions will always return success even if it is failed

Lines of code Vulnerability details Impact In NounsAuctionHouseFork.sol, safeTransferETH function is used to transfer ETH. File: nouns-contracts/contracts/governance/fork/newdao/NounsAuctionHouseFork.sol 272 function safeTransferETHaddress to, uint256 value internal returns bool 273 bool success,...

Admin user has an absolute power to withdraw all contract balance, which may raise red flags for investors

Lines of code Vulnerability details Impact Having rug-pull related code is always considered as a red flag for new investors. An admin, who's a single point of failure has access to withdraw function, which allows to withdraw the whole contract balance. Even if the owner is genuine the rug pull...

Execution of incomplete or mismatched actions due to arrays of different lengths

Lines of code Vulnerability details Impact an attacker can pass different-sized arrays for these parameters. For example, if the attacker passes an array of length 3 for targets, but arrays of length 2 for values, signatures, and calldatas, the function will still proceed and store the proposal...

initializer can be front run

Lines of code Vulnerability details Impact Initialize function have the potential of front running by a malicious actor. An attacker can front-run the deployer and takeover the contract by setting itself as the owner in the NounsTokenFork.sol Contract. Taking ownership will result in passing valu...

tx.origin may be removed in future and its usage is not recommended

Lines of code Vulnerability details Impact There is a chance that tx.origin will be removed from the Ethereum protocol in the future, so code that uses tx.origin must be avoid using it. There is also some EIPs being proposed for change/remove of tx.origin. ethereum/EIPs637 In NounsDAOLogicV2.sol,...

Storage collision risk in NounsDAOProxy contracts

Lines of code Vulnerability details Impact NounsDAOProxy contract may lose tracking its implementation address Proof of Concept One of the main vulnerabilities of upgradeable contracts is storing the implementation address in the beginning slots. This address is later used by proxy for delegateca...

The NounsDAOLogicV3.sol contract has a receive function but no withdraw function.

Lines of code Vulnerability details Impact All Ether sent to the NounsDAOLogicV3.sol will be locked in the contract because it implements a receive function without a withdraw function. Proof of Concept The NounsDAOLogicV3.sol contract implements the receive function but does not have a withdraw...