Minting to user after funds deposit is now omitted in Lender's Swivel lend

Lines of code Vulnerability details Lender's Swivel version of lend pulls the underlying funds from the user, opens the necessary positions, but now fails to mint Illuminate PT for the user, so there will be no records for her investment and the corresponding funds aren't recoverable. Setting...

No minting done in the Element version of lend function, user funds are frozen within the system

Lines of code Vulnerability details Lender's Element lend transfers the funds from a user, opens the position with Element, but fails to mint a corresponding Illuminate position to a user. Setting severity to be high as there is no account of user investment is effectively created, so there is no...

Yield of LiquidityReserve can be stolen

Lines of code Vulnerability details Impact Using sandwich attacks and JIT Just-in-time liquidity, the yield of LiquidityReserve could be extracted for liquidity providers. Proof of Concept The yield of LiquidityReserve is distributed when a user calls instantUnstakeReserve in Staking. Then, in...

Upgraded Q -> H from 104 [1656255316696]

Judge has assessed an item in Issue 104 as High risk. The relevant finding follows: L02: Incompatibility with ERC-4626 Line References Description The EIP-4626 specification requires that totalAssets to NOT revert, but the current implementation does so in the underlying methods: int256...

User to lose all the funds when lend() to Swivel

Lines of code Vulnerability details function lend uint8 p, address u, uint256 m, uint256 memory a, address y, Swivel.Order calldata o, Swivel.Components calldata s public unpausedp returns uint256 // lent represents the number of underlying tokens lent uint256 lent; // returned represents the...

No check for set with address(0)

Lines of code Vulnerability details Impact can mint and get approve from address 0. Proof of Concept function setMarketPlaceaddress m external authorizedadmin returns bool if marketPlace != address0 revert ExistsmarketPlace; marketPlace = m; return true; Tools Used None Recommended Mitigation Ste...

Removal of liquidity from the reserve can be griefed

Lines of code Vulnerability details Impact Users may be unable to withdraw/remove their liquidity from the LiquidityReserve if a user decides to grief the contract. Proof of Concept This is the only function in this contract that is able to unstake funds, so that they can be withdrawn/removed:...

Reentrancy vulnerabilities

Lines of code Vulnerability details Impact Reentrancy attacks can allow malicious actors to take over the control flow, and make...

[M-04] Admin can lose control on the contract

Lines of code Vulnerability details Proof of Concept The setAdmin function is too loose, if no one have access to the address passed in this function, the whole project is stuck - cannot withdraw funds or fees, cannot create new markets, etc. Recommended Mitigation Steps Change setAdmin in favour...

can override a marketplace

Lines of code Vulnerability details Impact There is no check if there is a market place in the m address and we can override all of the exist market places. Proof of Concept function setMarketPlaceaddress m external authorizedadmin returns bool if marketPlace != address0 revert ExistsmarketPlace;...

if attacker gets admin private key then he can drain all the tokens that Lender.sol contract holds

Lines of code Vulnerability details Impact if attacker gets admin private key then he can drain all the tokens that Lender.sol contract holds Recommended Mitigation Steps make it harder for admin to do this with some internal sec requirements --- The text was updated successfully, but these error...

removeAddress doesn't decrease the contracts.length

Lines of code Vulnerability details Impact The contracts length will always increase because the removeAddress function just deleting the value inside the array and never decrease the length by calling pop method. This can lead to Dos when calling functions that doing loop on contracts storage:...

Centralization Risk On The Withdraw Operation

Lines of code Vulnerability details Impact During the code review, It has been observed that admin can withdraw all tokens from the system. Proof of Concept 1. Navigate to the following contract : Tools Used Code Review Recommended Mitigation Steps We advise the client to carefully manage the adm...

Redeemer.sol#redeem() can be called by anyone before maturity, which may lead to loss of user funds

Lines of code Vulnerability details function redeem uint8 p, address u, uint256 m public returns bool // Get the principal token that is being redeemed by the user address principal = IMarketPlacemarketPlace.marketsu, m, p; // Make sure we have the correct principal if p !=...

token transfers in LiquidityReserve and Staking contract don't support deflationary ERC20 tokens, and user funds can be lost if stacking token was deflationary

Lines of code Vulnerability details Impact if the token is deflationary then contract will receive less token that requested amount but contract don't check for the real transferred amount. because this is happening in receiving stackingtoken in addLiquidity of LiquidityReserve and stake of Staki...

Functions in the BatchRequests contract revert for removed contract addresses

Lines of code Vulnerability details Impact Removing Yieldy contract addresses from the contracts array with BatchRequests.removeAddress replaces the contract address with a zero-address due to how delete works. Each function that loops over the contracts array or accesses an array item by index,...

[M-02] transferring LESS funds from lender than needed

Lines of code Vulnerability details Impact Wrong accounting calculation may result in malfunctioning and loss of funds for the project. Proof of Concept As seen in the lines above, the amount transferred from the lender is after subtracting the fee he actually needs to pay On the other hand, the...

Call to lend() function can be frontrunned with fee increase

Lines of code Vulnerability details Impact Malicious admin could frontrun users lend transaction anytime and set feenominator to any value using setFee and bigger fee than user expected. /// @notice sets the admin address /// @param a address of a new admin /// @return bool true if successful...

Illuminate PT redeeming do not return underlying to a user

Lines of code Vulnerability details In the second step of two step redeeming process, the underlying funds should be located in the Redeemer contract as external system burned the shares the Redeemer obtained from the Lender contract and returned underlying to Redeemer. Then Redeemer's Illuminate...

Upgraded Q -> H from 158 [1656140317935]

Judge has assessed an item in Issue 158 as High risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

Griefing Attack by staking for any receipent

Lines of code Vulnerability details Impact Any user can grief attack and lock funds of victim by staking with victim address as recipient. Proof of Concept If warmup period is more than 1, it is possible to do this by staking with victim address as recipient with 1 wei amount, and increasing the...

Upgraded Q -> H from 158 [1656140803140]

Judge has assessed an item in Issue 158 as High risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

Griefers can prevent users from claiming YIELDY_TOKENs from the warm up

Lines of code Vulnerability details Impact When warmUpPeriod is greater than 0, anyone can deny i.e.: grief any user from claiming their YIELDYTOKENs from Staking.sol. Proof of Concept 1. Admin sets the warmUpPeriod to 2. 2. A user deposits a decent amount STAKINGTOKENs, with the expectation that...

Staking: Attacker can stake very few tokens for others to increase the lock time of others' tokens.

Lines of code Vulnerability details Impact In the stake function of the Staking contract, anyone can stake tokens for others. And each time a token is staked, the lock time of all tokens is increased. This allows an attacker to stake few tokens for others to increase the lock time of others'...

No withdrawal possible for ETH TOKE pool

Lines of code Vulnerability details Impact The withdraw function of the ETH Tokemak pool has an additional parameter asEth. This can be seen in the Tokemak Github repository or also when looking at the deployed code of the ETH pool. Compare that to e.g. the USDC pool, which does not have this...

Denial of Service by wrong BatchRequests.removeAddress logic

Lines of code Vulnerability details Impact The BatchRequests.removeAddress logic is wrong and it will produce a denial of service. Proof of Concept Removing the element from the array is done using the delete statement, but this is not the proper way to remove an entry from an array, it will just...

sell() and buy() should not have whenNotPaused modifier

Lines of code Vulnerability details Impact Impacts are two fold: 1. admin should not be allowed to stop users from selling/cashing out their assets under any circumstance. 2. Due to poor timing or malicious admin activity, it could be the case that buy is put on pause right after initiateBuyout i...

Twav.sol#_getTwav() will revert when timestamp > 4294967296

Lines of code Vulnerability details function getTwav internal view returnsuint256 twav if twavObservationsTWAVBLOCKNUMBERS - 1.timestamp != 0 uint8 index = twavObservationsIndex + TWAVBLOCKNUMBERS - 1 % TWAVBLOCKNUMBERS; TwavObservation memory twavObservationCurrent = twavObservationsindex;...

NibblVault buyout duration longer than update timelock

Lines of code Vulnerability details Impact User can buy out NFT by initiating the process through initiateBuyout, then he has to wait BUYOUTDURATION which is 5 days and if the buyout will not get rejected he can claim the NFT. During that period bidder cannot cancel the process. The issue is that...

Issue with TWAV calculation.

Lines of code Vulnerability details Impact To calculate time weighted average value, current valuation is used. This is not appropriate way to decide the TWAV while other locations considers only time. This might not be the stable one. Proof of Concept uint256 currentValuation =...

Fee Was Not Charged When Buying On Secondary Curve

Lines of code Vulnerability details Proof-of-Concept Per the documentation, the admin and curator fees are charged when buying on the secondary curve. Whenever someone mints and burns tokens on the bonding curve, they need to pay some trading fees which is accrued in ETH Within the NibblVault.buy...

Malicious NFT

Lines of code Vulnerability details Impact Anybody can create a vault which fractionalizes their NFT. However, this process should go through a whitelist and review process because a maliciously implemented NFT contracts can reclaim ownership of the contract despite it technically being 'owned' b...

Buy logic is not correct

Lines of code Vulnerability details Impact This will affect the curves from where the buy need to happen. This will lead to erroneous result. Proof of Concept Tools Used Manual calculation and code review Recommended Mitigation Steps Update the Buy logic by referring the sell logic. --- The text...

_rejectBuyout() does not take buys and sells into account until next block, can lead to false effects due to timing

Lines of code Vulnerability details Impact Buyout that should be rejected will be allowed to happen. Proof of Concept Last user to call buy that could have pushed rejectBuyout to reject the buyout wont be accounted for because rejecBuyout is place before the minting of new tokens. Therefore even ...

_getTwav is not correct. It is not considering all last 4 blocks.

Lines of code Vulnerability details Impact This will result in invalid deadline to decide the buyout end. Proof of Concept /// @notice returns the TWAV of the last 4 blocks /// @return twav TWAV of the last 4 blocks function getTwav internal view returnsuint256 twav if...

Discrepancies between document and code implementation

Lines of code Vulnerability details Impact This will tarnish the credibility of the system. Proof of Concept Document says buyout time as 3 days. But the code is implemented with 5 days as buyout time. Tools Used Manual verification Recommended Mitigation Steps Implement the code as per what...

Malicious Basket Could Be Used To Rug Fractionalized Token Holders

Lines of code Vulnerability details Proof-of-Concept The NibblVaultFactory.createVault accepts any NFT address or any Basket address. Therefore, an attacker could pass a malicious basket address during vault creation. function createVault address assetAddress, address curator, string memory name,...

Buyer can initiateBuyout by sending less value than current valuation

Lines of code Vulnerability details Impact Intuitively, a buyer calling initiateBuyout should be sending a msg.value greater than or equal to current valuation. However, they can actually send less than this amount and still initiate the buyout. On line 404 we have requirebuyoutBid =...

Proxy storage collision - contracts might be bricked

Lines of code Vulnerability details The proxy contracts save the implementation variables in the first storage slot see links above. This will lead to storage collusions when the implementation contract tries to read/update the first storage slot. For the implementation contract, the first storag...

Buys/sells will be front-run by MEV bots

Lines of code Vulnerability details Impact Since the value of the vault depends on the Bancor formula, it's important that transactions can't be front-run, because doing so, even for small amounts, will change the valuation price for subsequent buys/sells. The link the sponsor provided describes...

sell function does not have minimum threshold on the _minAmtOut

Lines of code Vulnerability details Impact During the code review, It has been observed saleReturn value is not resistant to slippage on the minAmtOut. For the protecting value from slippage, the protocol should ensure that minAmtOut is different than zero. Proof of Concept Tools Used Recommended...

Avoid using address(this).balance for internal accounting

Lines of code Vulnerability details Impact This could reduce or increase the amount of ETH that token-holders are able to redeem for a reserve token in the case of a buyout success. Proof of Concept Using addressthis.balance for any internal accounting for smart contract poses risks. Using...

[PNM-004] Calculation of _secondaryReserveRatio can be overflowed

Lines of code Vulnerability details Description uint32 secondaryReserveRatio = uint32msg.value SCALE 1e18 / initialTokenSupply initialTokenPrice; secondaryReserveRatio can be overflowed by setting a relatively small initialTokenSupply and initialTokenPrice. The result will be truncated by uint32,...

ProxyVault implementation can be changed retroactively bricking old NibblVaults

Lines of code Vulnerability details Impact The ProxyVault.sol contract refers back to the NibbleVaultFactory.vaultImplementation This value can change over time, potentially breaking storage of the old implementations. Notice that querying the implementation from NibbleVaultFactory also incurs a...

[PNM-002] Unfairness: the curator (or users with large shares) can always reject payout by sandwich attacks

Lines of code Vulnerability details Impact 15% Attack Description The initiateBuyout function uses the current valuation instead of the time-weighted average one to determine buyoutRejectionValuation, making the curator or any other early buyers with a relatively large sharing able to reject...

NibblVault permit functionality allows redeeming only one "active" signature

Lines of code Vulnerability details Impact Contract NibblVault implements function permit that allows approving spender to spend value of tokens that belongs to owner. The issue is that structHash keeps incrementing noncesowner++ which in case of multiple "active" permits signatures of the same...

Lack of lock modifier on sell() can lead to reentrancy

Lines of code Vulnerability details Impact NibblVault.sell does not possess the lock modifier that protects against reentrancy. The only attack vector due to reentrancy in this function was found to be executable by the factory that receives the admin fees which is not plausible due to the curren...

REENTRANCY-Avoid transfer() as reentrancy mitigations. Transfer do not protect from reentrancies in case of gas price changes.

Lines of code Vulnerability details Impact --Check: reentrancy-unlimited-gas --Severity: Informational --Confidence: Medium Transfer and send have been recommended as a security best-practice to prevent reentrancy attacks because they only forward 2300 gas. That being said, gas repricing of...

Division by 0

Lines of code Vulnerability details Division by 0 can lead to accidentally revert, An example of a similar issue - code-423n4/2021-10-defiprotocol-findings84 Code instances: https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.solL183 initialTokenSupply, initialTokenPrice...

User Could Change The State Of The System While In Pause Mode

Lines of code Vulnerability details Proof-of-Concept Calling NibblVault.updateTWAP function will change the state of the system. It will cause the TWAP to be updated and buyout to be rejected in certain condition. When the system is in Pause mode, the system state should be frozen. However, it wa...