Function may not be implemented

Handle jayjonah8 Vulnerability details Impact In StabilizerNode.sol the distributeSuply function calls the check function on the ISupplyDistributionController. Searching through the code base I didn't location where the function is actually implemented. If this is calling a function within the...

Purchased Malt and Auction Data Can Be Manipulated/Thrown Off

Handle jayjonah8 Vulnerability details Impact In Auction.sol, the purchased variable in the purchaseArbitrageTokens function can be manipulated throwing off the AuctionData and the ratio of the realCommitment vs the purchased amount. This is because "purchased" simply returns...

Inaccurate fees computation

Handle cmichel Vulnerability details The MixinTransfer.shareKey function wants to compute a fee such that time + fee time == timeRemaining timePlusFee: uint fee = getTransferFeekeyOwner, timeShared; uint timePlusFee = timeShared + fee; However, if the time remaining is less than the computed fee...

MEV miner can mint larger than expected UDT total supply

Handle elprofesor Vulnerability details Impact UnlockProtocol attempts to calculate gas reimbursement using tx.gasprice, typically users who falsify tx.gasprice would lose gas to miners and therefore not obtain any advantage over the protocol itself. This does present capabilities for miners to...

Malicious user can get infinite free trial by repeatedly refund and repurchase right before the freeTrial ends

Handle WatchPug Vulnerability details The current design/implementation allows users who are refunded before to get another freeTrial. This can be exploited by malicious users to get an infinite free trial. PoC Given: keyPrice is 1 ETH; freeTrialLength is 31 days. A malicious user can: 1. Call...

Frontrunning PublicLock.initialize() can prevent upgrades due to insufficient access control

Handle elprofesor Vulnerability details Impact The unlock protocols base contract Unlock.sol uses setLocktemplate to initialize the implementation contract for the PublicLock proxy. This function will initialize the relevant PublicLock contract which has been deployed separately...

New lock version might be initialized by malicious actor

Handle kenzo Vulnerability details Unlock can add a new lock version, which lock owners can upgrade to, even before the implementation has been initialized. So anybody could initialize it before Unlock does. Impact If somebody else has initialized the new lock version before Unlock has called...

Referrer discount token amount can be manipulated

Handle cmichel Vulnerability details The Unlock.recordKeyPurchase function is called on each key purchase MixinPurchase.purchase and mints UDT tokens to the referrer. The amount to mint is based on the transaction's gas price which is controlled by the caller purchaser: uint tokensToDistribute =...

Key self transfers lead to expiry

Handle cmichel Vulnerability details The MixinTransfer.shareKey function does not disallow transfering the key to oneself, i.e. from == recipient. When doing a self-transfer, the remaining time for oneself should be reduced by the fees, but the keys immediately expire due to explicitly setting th...

Wrong design/implementation of freeTrial allows attacker to steal funds from the protocol

Handle WatchPug Vulnerability details The current design/implementation of freeTrial allows users to get full refund before the freeTrial ends. Plus, a user can transfer partial of thier time to another user using shareKey. This makes it possible for the attacker to steal from the protocol by...

MixinPurchase:shareKey allows to generate keys without purchasing

Handle GiveMeTestEther Vulnerability details Impact the shareKey function allows a user to share some time with another user that doesn't already has/had a key and this generates a new key. This even allows to generate more keys than maxNumberOfKeys. attacker generates a lot of EOA adresses, buys...

MixinTransfer.sol#transferFrom Wrong implementation can potentially allows attackers to reverse transfer and cause fund loss to the users

Handle WatchPug Vulnerability details if toKey.tokenId == 0 toKey.tokenId = tokenId; recordOwnerrecipient, tokenId; // Clear any previous approvals clearApprovaltokenId; if previousExpiration = block.timestamp // The recipient did not have a key, or had a key but it expired. The new expiration is...

Change of freeTrialLength, refundPenaltyBasisPoints and expirationDuration can cause unexpected results to users

Handle WatchPug Vulnerability details function getCancelAndRefundValue address keyOwner private view hasValidKeykeyOwner returns uint refund Key storage key = keyByOwnerkeyOwner; // Math: safeSub is not required since hasValidKey confirms timeRemaining is positive uint timeRemaining =...

Missing scaling factor in recordKeyPurchase?

Handle cmichel Vulnerability details The Unlock.recordKeyPurchase function computes the maxTokens as: maxTokens = IMintableERC20udt.balanceOfaddressthis valueInETH / 2 + 2 valueInETH / grossNetworkProduct / grossNetworkProduct; Note that grossNetworkProduct was already increased by valueInETH in...

Approvals not cleared after key transfer

Handle cmichel Vulnerability details The locks implement three different approval types, see onlyKeyManagerOrApproved for an overview: key manager map keyManagerOf single-person approvals map approved. Cleared by clearApproval or setKeyManagerOf operator approvals map managerToOperatorApproved Th...

MixinRefunds: frontrun updateKeyPricing() for free profit

Handle itsmeSTYJ Vulnerability details Impact A malicious user is able to withdraw all payments that were paid to a lock owner if the owner increases the keyPrice. Proof of concept When updateKeyPricing is called to increase the price of a key, it is possible to frontrun this call and buy many ke...

Unlock: free UDT arbitrage opportunity

Handle itsmeSTYJ Vulnerability details Impact Uniswap v2 made oracle attacks much more expensive to execute since it needs to be manipulated over X number of blocks however its biggest drawback is that it reacts slow to price volatility depends on how far back you look. Depending on a single orac...

keyPrice is global and change on keyPrice can damage lockManager or keyOwner

Handle 0x0x0x Vulnerability details Impact A lock manager changes keyPrice. If the price is increased, then possible refunds are also increased and it can result in theft of funds. If the price is reduced, by doing so lock manager can reduce the refund payments and scam the users. Tools Used Manu...

MixinTransfer.sol#shareKey charges extra when timePlusFee >= timeRemaining.

Handle 0x0x0x Vulnerability details Impact Users can lose extra time, when they shareKey Proof of Concept When timePlusFee timeRemaining does not hold, the fee is calculated as follow: fee = getTransferFeekeyOwner, timeRemaining; time = timeRemaining - fee; By doing so fee is also charged for the...

Key transfer will destroy key if from==to

Handle kenzo Vulnerability details If calling transferFrom with from == recipient, the key will get destroyed meaning the key will be set as expired and set the owner's key to be 0. Impact A key manager or approved might accidently destroy user's token. Note: this requires user error and so I'm n...

transferFrom() and shareKey() doesn't check for _from != _recipient

Handle GiveMeTestEther Vulnerability details Impact / POC There is no check from != recipient in transferFrom https://github.com/code-423n4/2021-11-unlock/blob/ec41eada1dd116bcccc5603ce342257584bec783/smart-contracts/contracts/mixins/MixinTransfer.solL109 the key manager/approver can expire the k...

Fees can be more than 100%

Handle cmichel Vulnerability details The Overlayv1Mothership.adjustGlobalParams function allows setting a fee and feeBurn that is greater than 100%. Recommended Mitigation Steps Validate that fee and feeBurn is less than ONE = 1e18. --- The text was updated successfully, but these errors were...

_totalSupply not updated in _transferMint() and _transferBurn()

Handle gpersoon Vulnerability details Impact The functions transferMint and transferBurn of OverlayToken.sol don't update totalSupply. Whereas the similar functions mint and burn do update totalSupply. This means that totalSupply and totalSupply will not show a realistic view of the total OVL...

Fee boundaries are not validated

Handle pauliax Vulnerability details Impact OverlayV1Mothership has declared variables named MINFEE and MAXFEE, or MINMARGINMAINTENANCE and MAXMARGINMAINTENANCE, however, none of these variables are used anywhere, e.g. I expected to see a fee validated with these min/max boundaries, but now...

User can refund the received airdropped (free) "key"

Handle GiveMeTestEther Vulnerability details Impact User can refund the received airdropped free "key". If a lot of users received a airdropped "key" they can drain the funds of the lock. e.g. key owner cannot withdraw anything, user that purchased a key can't cancel and refund. Proof of Concept ...

Can't enableCollateral after a disableCollateral

Handle gpersoon Vulnerability details Impact The function disableCollateral of OverlayV1Mothership.sol doesn't set collateralActivecollateral = false; But it does revoke the roles. Now enableCollateral can never be used because collateralActivecollateral ==true and it will never pass the second...

Unbounded marginMaintenance

Handle gzeon Vulnerability details Impact There is no check for the value of marginMaintenance in OverlayV1OVLCollateral.sol setMarketInfo despite apparently related MINMARGINMAINTENANCE and MAXMARGINMAINTENANCE defined but unused in OverlayV1Mothership.sol. marginMaintenance can be set to 100% t...

Key buyers will not be able to get refund if lock manager withdraws profits

Handle kenzo Vulnerability details Unlock contains a feature in which a key buyer can ask for a refund. The refund is sent from the lock - where the purchase funds were sent. The lock manager can withdraw all funds from the lock. Therefore, if the lock manager withdraws enough profits from the...

_transferMint not incrementing totalSupply of OVL after minting

Handle hubble Vulnerability details Impact The function transferMint is called by both transferFromMint and transferMint which again is used while issuing shares when a user takes a position in OI. Proof of Concept File : ovl/OverlayToken.sol file line 268 : function transferMint address sender,...

OZ ERC1155Supply vulnerability

Handle pauliax Vulnerability details Impact Overlay uses OZ contracts version 4.3.2: dependencies: - OpenZeppelin/[email protected] and has a contract that inherits from ERC1155Supply: contract OverlayV1OVLCollateral is ERC1155Supply This version has a recently discovered vulnerability...

_totalSupply not updated on transferMint/Burn

Handle cmichel Vulnerability details The OverlayToken has a transferMint and transferBurn function which is supposed to act like a transfer followed by a mint/burn. However, a mint/burn updates the totalSupply see mint/burn but these functions do not. Recommended Mitigation Steps The transferMint...

Improper Upper Bound Definition on the Fee

Handle defsec Vulnerability details Impact In the adjustGlobalParams function on line 1603of "", adjustGlobalParams function does not have any upper or lower bounds. Values that are too large will lead to reversions in several critical functions. Proof of Concept The setFee function that begins o...

OverlayV1Mothership.sol Incomplete implementation

Handle WatchPug Vulnerability details Based on the context, FEE and MARGINMAINTENANCE rates should be bounded. However, the current implementation does not enforce these bounds. uint16 public constant MINFEE = 1; // 0.01% uint16 public constant MAXFEE = 100; // 1.00% uint16 public constant...

Unbounded fee

Handle gzeon Vulnerability details Impact There is no check on the value of fee in OverlayV1Mothership.sol despite MINFEE and MAXFEE has been defined in L10. This issue is considered to be higher risk than the other because this enable the possibility for governance to "rug" users' fund by settin...

_transferBurn should reduce totalSupply

Handle WatchPug Vulnerability details function transferBurn address sender, address recipient, uint256 amount, uint256 burnt internal uint256 senderBalance = balancessender; requiresenderBalance = amount + burnt, "OVL:balance= amount + burnt, "OVL:balanceamount+burnt"; unchecked balancessender =...

Missing maxNumberOfKeys checks in shareKey and grantKey

Handle kenzo Vulnerability details More keys can be minted than maxNumberOfKeys since shareKey and grantKey do not check if the lock is sold out. Impact More keys can be minted than intended. Proof of Concept In both shareKey and grantKey, if minting a new token, a new token is simply minted and...

_transferMint does not increase _totalSupply, which can malfunction burn()

Handle WatchPug Vulnerability details function transferMint address sender, address recipient, uint256 amount, uint256 minted internal uint256 senderBalance = balancessender; requiresenderBalance = amount, "ERC20: transfer amount exceeds balance"; unchecked balancessender = senderBalance - amount...

OverlayV1UniswapV3Market computes wrong market liquidity

Handle cmichel Vulnerability details The OverlayV1UniswapV3Market.fetchPricePoint tries to compute the market depth in OVL terms as marketLiquidity in ETH / ovlPrice in ETH per OVL. To get the market liquidity in ETH and not the other token pair, it uses the ethIs0 boolean. marketLiquidity = ethI...

Fee double counting for underwater positions

Handle hyh Vulnerability details Impact Actual available fees are less than recorded. That's because a part of them corresponds to underwater positions, and will not have the correct amount stored with the contract: when calculation happens the fee is recorded first, then there is a check for...

pow() is missing check on input parameters with 0 value

Handle gpersoon Vulnerability details Impact The contract LogExpMath.sol seems to be a fork of the balancer LogExpMath.sol contract. It is mostly similar, except for checks for x and y being 0 in the beginning of the function pow, see below. This omission might lead to unexpected results. Proof o...

isUnderwater returns opposite boolean for short positions

Handle harleythedog Vulnerability details Impact The function isUnderwater should return true iff the position value is 0. In the case of a short position, this is when oi 2 - priceFrame - debt 0 based on the logic given in the value function. Rearranging this equation, a short position is...

Using/Importing ERC1155Supply.sol from openzeppelin version 4.3.2

Handle hubble Vulnerability details Using/importing openZeppelin version 4.3.2 file ERC1155Supply.sol which has a security advisory. Reference : GHSA-wmpv-c2jp-j2xg Proof of Concept File : collateral/OverlayV1OVLCollateral.sol line 7: import...

No bound on marginRewardRate

Handle gzeon Vulnerability details Impact There is no check when setting marginRewardRate, if it is set to any value FixedPoint.ONE i.e. 1e18 it would lead to underflow at L403 Proof of Concept Recommended Mitigation Steps Add related check in setMarketInfo --- The text was updated successfully,...

Malicious user can DOS lock by falsely reaching maxNumberOfKeys using shareKey

Handle kenzo Vulnerability details A malicious user can call shareKey repeatedly, transferring miniscule amounts of his key to different accounts, thereby minting new keys until maxNumberOfKeys is reached. Impact Malicious user can grief and make lock purchasing become disabled. Proof of Concept ...

Timelock and events for governor functions

Handle pauliax Vulnerability details Impact There are contracts that contain functions that change important parameters of the system, e.g. OverlayV1Mothership has setOVL, initializeMarket, disableMarket, enableMarket, initializeCollateral, enableCollateral, disableCollateral, adjustGlobalParams...

Cached version of ovl may be outdated

Handle pauliax Vulnerability details Impact contract OverlayV1OVLCollateral and OverlayV1Governance cache ovl address: IOverlayTokenNew immutable public ovl; This variable is initialized in the constructor and fetched from the mothership contract: mothership = IOverlayV1Mothershipmothership; ovl ...

_transferBurn not decrementing totalSupply of OVL after burn

Handle hubble Vulnerability details Impact The function transferBurn is called by both transferFromBurn and transferBurn which again is used when unwinding or liquidation of a position in OI. Proof of Concept File : ovl/OverlayToken.sol file line 194 : function transferBurn address sender, addres...

Unbounded feeBurnRate and marginBurnRate

Handle gzeon Vulnerability details Impact There are no checks when setting feeBurnRate and marginBurnRate in OverlayV1Mothership constructor and adjustGlobalParams, if one of the 2 variable is set to any value FixedPoint.ONE i.e. 1e18 it would lead to underflow at OverlayV1OVLCollateral:L147 and...

Refund mechanism doesn't take into account that key price can change

Handle kenzo Vulnerability details Lock manager can change key pricing. The refund mechanism calculates refund according to current key price, not price actually paid. Impact A user refunding can get less or more funds than deserved. Proof of Concept Refund only takes the current price into...

Total Supply Does Not Change During the Burn/Mint

Handle defsec Vulnerability details Impact During the dynamic test, The Burn and Mint function does not increase/decrease total supply. That will cause liquidity loss on the protocol. Proof of Concept 1. Navigate to the following contracts. "" "" 2. Totalsupply is not adjusted according to...