initializeMarket always initialize the latest market

Handle jonah1005 Vulnerability details Impact In longshore contract's initializeMarket, while it should initialize according to the parameter marketIndex, it initialize latestMarket. This would break two market, the market of marketIndex' and market of latestMarket. User's fund would get stuck at...

Single-step process for critical ownership transfer/renounce is risky

Handle 0xRajeev Vulnerability details Impact The SwappableYieldSource allows owners and asset managers to set/swap/transfer yield sources/funds. As such, the contract ownership plays a critical role in the protocol. Given that AssetManager is derived from Ownable, the ownership management of this...

reputation risk via upgradable contracts

Handle gpersoon Vulnerability details Impact The contract SwappableYieldSource is upgradable. This means the owner could upgrade and change the contract so any new functionality. Amongst others the owner could retrieve all the tokens of the Yieldsource and transfer them out. The project could sti...

Transfer-on-fee/deflationary tokens are not correctly accounted for

Handle shw Vulnerability details Impact When a user stakes or a protocol deposits a transfer-on-fee/deflationary token, the solution does not correctly handle the received amount, which could be less than what is accounted for. Proof of Concept Referenced code: PoolOpen.solL36-L38...

Difficult for the project to be decentralized if the Watsons share one address.

Handle tensors Vulnerability details Impact The Watsons share a single address. As it stands right now the Watsons could be a single person effectively providing insurance with other peoples risk. There should be mechanisms in place to make sure Watson's have an accurate amount of skin in the gam...

Single under-funded protocol can break paying off debt

Handle cmichel Vulnerability details The SherXERC20.payOffDebtAll function iterates over all protocols of the token. If a single project does not have enough funds to cover the premium payments, the transactions come to a halt, see payOffDebt: debt = accruedDebtps, protocol, blocks; // this can...

Router.swapSynthToAsset(uint256,address,address) has unchecked transfer

Handle heiho1 Vulnerability details Impact Router.swapSynthToAssetuint256,address,address on line 249 ignores the boolean return on transfers. This is a brittle implementation because it relies on the boolean return value being hard-coded to true. Tokens may return false and not revert in which...

Pool._addPoolMetrics(uint256) is subject to potential miner manipulation

Handle heiho1 Vulnerability details Impact Pool.addPoolMetricsuint256 on line 334 relies on block.timestamp and is potentially vulnerable to miner manipulation. This could lead to erroneous pool metrics. Proof of Concept Tools Used Slither Recommended Mitigation Steps An external time oracle like...

addLiquidity transfers tokens from the wrong sender

Handle pauliax Vulnerability details Impact In addLiquidity function, a router is passed as a sender in LibERC20.transferFrom, not msg.sender, so it basically transfers assets from the router to the contract. Recommended Mitigation Steps requireLibERC20.transferFromassetId, msg.sender, addressthi...

Logic error in fulfill() function

Handle s1m0 Vulnerability details The fulfill function could create a loss for the protocol in case addFunds succeed and execute function fails. Impact In the fulfill function the toSend amount could be transferred twice: 1. In the external call addFunds to the txData.callTo. 2. If the execute...

Signatures use only tx ID instead of entire digest

Handle 0xRajeev Vulnerability details Impact The signature check in recoverFulfillSignature only uses transaction ID along with the relayer fee which can be accidentally reused by the user, in which case the older signatures with the older relayer fees can be replayed. The signature should be on...

Flash loan risk mitigation is optional and not robust enough

Handle 0xRajeev Vulnerability details Impact The switchEoaOnly allows the owner to disable preventSmartContracts the project’s plan apparently is to do so after the beta-period which will allow any smart contract to interact with the protocol and potentially exploit any underlying flash loan...

Buoy3Pool.safetyCheck is not precise and has some assumptions

Handle cmichel Vulnerability details Vulnerability Details The safetyCheck function has several issues that impact how precise the checks are: 1. only checks if the a/b and a/c ratios are within BASISPOINTS. By transitivity b/c is only within 2 BASISPOINTS if a/b and a/c are in range. For a more...

sortVaultsByDelta doesn't work as expected

Handle gpersoon Vulnerability details Impact The function sortVaultsByDelta doesn't always work as expected. Suppose all the delta's are positive, and delta1 = delta2 = delta3 0 Then maxIndex = 0 And delta minDelta ==0 is never true, so minIndex = 0 Then assuming bigFirst==true: vaultIndexes0 =...

Return values of ERC20 transfer and transferFrom are unchecked

Handle shw Vulnerability details Impact In some contracts e.g., TracerPerpetualSwaps.sol, the return values of ERC20 transfer and transferFrom are not checked to be true, which could be false if the transferred tokens are not ERC20-compliant. In that case, the transfer fails without being noticed...

Wrong price scale for GasOracle

Handle cmichel Vulnerability details The GasOracle uses two chainlink oracles GAS in ETH with some decimals, USD per ETH with some decimals and multiplies their raw return values to get the gas price in USD. However, the scaling depends on the underlying decimals of the two oracles and could be...

Trader orders can be frontrun and users can be denied from trading

Handle cmichel Vulnerability details The Trader contract accepts two signed orders and tries to match them. Once they are matched and become filled, they can therefore not be matched against other orders anymore. This allows for a griefing attack where an attacker can deny any other user from...

executeTrade can be frontrun

Handle gpersoon Vulnerability details Impact An attacker could monitor the mempool and see an executeTrade transaction. He then could checkout the parameters and see if a better trade is possible for himself. He might even create and sign a new trade and then submit the new trade via an...

YearnV2YieldSource wrong subtraction in withdraw

Handle cmichel Vulnerability details YearnV2YieldSource.withdrawFromVault uses a wrong subtraction. When withdrawing from the vault one redeems yTokens for tokens, thus the token balance of the contract should increase after withdrawal. But the contract subtracts the currentBalance from the...

User could deposit for free

Handle s1m0 Vulnerability details Impact deposit function doesn't check the return value of transferFrom that means if the erc20 token return false instead of reverting the user could deposit for free. Tools Used Manual analysis Recommended Mitigation Steps Use openzeppelin's SafeERC20 library. -...

Malicious user can trigger another user’s removal

Handle 0xRajeev Vulnerability details Impact By allowing anyone to call removeUserFromOrderbook instead of only Market::newRental or Treasury::withdrawDeposit or collectRentUser which may result in foreclosures and hence may need to trigger user removal, a malicious user can trigger another's...

Problems with non-standard compliant ERC20 tokens

Handle cmichel Vulnerability details Vulnerability Details Some tokens like USDT don't correctly implement the EIP20 standard and their transfer/transferFrom function return void instead of a success boolean. Calling these functions with the correct EIP20 function signatures will always revert...

maxSumOfPrices check is broken

Handle 0xRajeev Vulnerability details Impact rentAllCards requires the sender to specify a maxSumOfPrices parameter which specifies “limit to the sum of the bids to place” as specified in the Natspec @param comment. This is apparently for front-run protection. However, this function parameter...

transferFrom result not checked

Handle gpersoon Vulnerability details Impact The function deposit of SafeERC20.sol relies on the fact that transferFrom will revert if it can't transfer the erc20 tokens. However, depending on the ERC20 token, this doesn't happen and you have to check the result of transferFrom. With the wrong...

Join Factory Contract Replacement

Handle 0xsomeone Vulnerability details Impact The JoinFactory contract is utilizing the create2 OPCODE via syntactic sugar to deploy a new Join instance, however, no sanitization occurs on the inputs allowing contracts and thereby ownerships to be replaced at will. Proof of Concept If the...

stir to self

Handle gpersoon Vulnerability details Impact The function stir of Cauldron.sol can be manipulated when from == to. In that case the balance of "to" is increased while the balance of "from" isn't decreased. This is due to the fact that a temporary variable is used and the balance of "to" overwrite...

ChainLink price data could be stale

Handle cmichel Vulnerability details Vulnerability Details There is no check in FSDNetwork.getEtherPrice if the return values indicate stale data. This could lead to stale prices according to the Chainlink documentation: under current notifications: "if answeredInRound roundId could indicate stal...

addRegistrationTributeGovernance shoud call_addGovernanceTribute ?

Handle gpersoon Vulnerability details Impact The function addRegistrationTributeGovernance makes a call to addTribute, the same as addRegistrationTribute is doing However a function addGovernanceTribute also exists and this function is never called. It seem more logical that...

Reward rates can be changed through flash borrows

Handle @cmichelio Vulnerability details Vulnerability Details The rewards per market are proportional to their totalBorrows which can be changed by a large holder who deposits lots of collateral, takes out a huge borrow in the market, updates the rewards, and then unwinds the position. They'll on...

Incorrect liquidity unit calculation in Utils.sol

Handle 0xRajeev Vulnerability details Impact As per code comments, the calcLiquidityUnits function is supposed to calculate: // units = P t B + T b/2 T B slipAdjustment // P part1 + part2 / part3 slipAdjustment While part1, part2 and part3 are calculated correctly, they are combined as: uint unit...

deploySynth does not prevent token to be VADER or USDV

Handle paulius.eth Vulnerability details Impact function deploySynth checks that token is not VADER or not USDV. The condition should be && not || as OR condition always holds when VADER != USDV: function deploySynthaddress token external requiretoken != VADER || token != USDV;...

Tokens can be stolen through transferTo

Handle @cmichelio Vulnerability details Vulnerability Details I know that it's stated that: VADER, USDV, SYNTHS all employ the transferTo function, which interrogates for tx.origin and skips approvals. The author does not subscribe to the belief that this is dangerous In my opinion, it can be ver...

Vader functions can be called before initialization in init() of Vader.sol

Handle 0xRajeev Vulnerability details Impact All the external/public functions of Vader.sol can be called by other contracts even before Vader.sol contract is initialized. This can lead to exceptions, state corruption or incorrect accounting in other contracts, which may require redeployment of...

Isolated margin contracts declare but do not set the value of liquidationThresholdPercent

Email address [email protected] Handle paulius.eth Eth address 0x523B5b2Cc58A818667C22c862930B141f85d49DD Vulnerability details CrossMarginTrading sets value of liquidationThresholdPercent in the constructor: liquidationThresholdPercent = 110; Isolated margin contracts declare but do not set the...

No default liquidationThresholdPercent

Email address [email protected] Handle @cmichelio Eth address 0x6823636c2462cfdcD8d33fE53fBCD0EdbE2752ad Vulnerability details The IsolatedMarginTrading contract does not define a default liquidationThresholdPercent which means it is set to 0. The belowMaintenanceThreshold function uses this value...

No entry checks in crossSwap[Exact]TokensFor[Exact]Tokens

Email address [email protected] Handle gpersoon Eth address gpersoon.eth Vulnerability details The functions crossSwapTokensForExactTokens and crossSwapExactTokensForTokens of MarginRouter.sol do not check who is calling the function. They also do not check the contents of pairs and tokens They...

Example finding from form

Email address [email protected] Handle adamavenir Eth address 234234234 Vulnerability details Some details: detailsschmetails Impact Brace for it! Proof of concept proof of concept Tools used I used no tools. Just this form and my BARE HANDS Recommended mitigation steps I would recommend not doing...

Upgraded Q -> 2 from #37 [1703589922038]

Judge has assessed an item in Issue 37 as 2 risk. The relevant finding follows: L-02 openPosition maybe underflow in openPosition - Base.swap function openPosition DataStruct.OpenPositionParams calldata params public override nonReentrant returns uint96 lienId, uint256 collateralTo...

An artPiece with a creator address set to address(0) leads to a pseudo-permanent DoS if it ends up being auctioned.

Lines of code Vulnerability details Vulnerability details Description There is a potential for a DoS in the AuctionHouse contract related to the buyToken call from the ERC20TokenEmitter. This is due to the buyToken using the mint function to all the addresses that are stored on the creators array...

collateralizeWLp can be bypassed even when collateralization is paused

Lines of code Vulnerability details Impact Admin can pause collateralization for a specific mode to prevent users from providing more collateral either via collateralize or collateralizeWLp. However, due to not properly using internal accounting when tracking wLP collateral, users can still provi...

Using addPremium() to evade part of the fees from marginFrom

Lines of code Vulnerability details Vulnerability details When openPosition, we will charge a certain fee, the calculation formula is as follows: marginFrom + amountFromBorrowed FEEFACTOR / Base.BASISPOINT It will include marginFrom, which is mainly used to ensure enough collateralTo after swap,...

Validate Creators Array

Lines of code Vulnerability details Potential Risk: The validateCreatorsArray function in the CreatorUtils contract performs input validation for the creatorArray parameter, which is an array of CreatorBps structs. While the function checks the individual elements of the array for valid addresses...

Increase liquidity in close position may not cover original borrowed liquidity

Lines of code Vulnerability details Summary When a position is closed, there is no check to ensure that the effective added liquidity covers the original borrowed liquidity from the LP. Impact Closing a position in the Particle LAMM protocol must ensure that the borrowed liquidity gets fully adde...

Array is push()ed but not pop()ed, and is iterated over

Lines of code 96, 485, 485, 485, 485, 485, 485, 485, 485, 485, 485https://github.com/Tapioca-DAO/ta...

Interest still accuring when repayment is paused, creating debt that cannot be repaid

Lines of code Vulnerability details Impact Interest still accuring when repayment is paused Proof of Concept When the admin pause the lending pool repayment, as timestamp elapses, interest still accuring /// @inheritdoc ILendingPool function accrueInterest public uint lastAccruedTime =...

Small positions are allowed in the system that are not profitable for liquidators

Lines of code Vulnerability details Proof of Concept When someone borrows, then he can borrow any assets amount that he would like. In the end function will check that position is healthy, which means that user has enough collateral amount to cover borrowed amount. The problem is that this functi...

_safeMint() should be used rather than _mint() wherever possible

Lines of code 139, 178, 204 Vulnerability details mint is discouraged in favor of safeMint which ensures that the recipient is either an EOA or implements IERC721Receiver. Both OpenZeppelin and solmate have versions of this function. In the cases below, mint does not call...

Unsafe use of approve() with IERC20

Lines of code 321, 215, 184, 450, 761, 217, 157, 234, 339, 386https://github.com/Tapioca-DAO/t...

Wrapping ether with Ocean.doMultipleInteractions() reverts

Lines of code Vulnerability details Summary Wrapping ether with doMultipleInteractions reverts. This is the case if one or more of the interactions is handling ether. If we look at the internal helper function, doMultipleInteractions, it includes the following: if msg.value != 0...

Potential Issues with Address Casting and Validation in _calculateOceanId Function

Lines of code Vulnerability details Impact The use of abi.encodePacked without padding might introduce ambiguity in situations where input lengths are not fixed. Additionally, assuming tokenId can be any uint256 value without enforcing constraints could lead to unexpected behavior if constraints...