Number of prizes math is wrong

Handle cmichel Vulnerability details The math described in Splitting the prizes and implemented in DrawCalculator.numberOfPrizesForIndex seems to be wrong. Assuming a bit range of 4 16 possibilities per position and cardinality of 8. Note that degree is determined by the first position where it...

ConcentratedLiquidityPoolManager uses wrong index for incentive

Handle cmichel Vulnerability details The ConcentratedLiquidityPoolManager uses the positionId as an index for incentivespoolpositionId when it should be incentiveId instead: // @audit should be Incentive memory incentive = incentivespoolincentiveId; Incentive memory incentive =...

Two-step change of a swivel address

Handle pauliax Vulnerability details Impact MarketPlace allows an admin to change swivel to a different address. This function has no validations, even a simple check for zero-address is missing, and there is no validation of the new address being correct. If the admin accidentally uses an invali...

uToken ERC20 approve method missing return value check #L109

Handle defsec Vulnerability details Impact The initiateVaultFillingZcTokenInitiate function performs an ERC20.approve call but does not check the success return value. Some tokens do not revert if the approval failed but return false instead. Proof of Concept 1. Navigate to "" 2...

Prevent creating the same market twice

Handle gpersoon Vulnerability details Impact The function createMarket of MarketPlace.sol doesn't check if the market already exists. So it could accidentally deploy a market with has the same maturity timestamp twice and overwrite the previous values of the market. The previously deployed market...

Erc20.transferFrom() return value is ignored

Handle pants Vulnerability details According to the ERC-20 Token Standard, the function transferFrom returns false on failure. However, the return value of Erc20.transferFrom is ignored 11 times: 1. In Swivel.initiateVaultFillingZcTokenInitiate, line 103. 2. In...

Missing timelock for critical contract setters of privileged roles

Handle 0xRajeev Vulnerability details Impact Setter functions for critical protocol parameters accessible only by privileged roles e.g. onlyOwner should consider adding timelocks so that users and other privileged roles in the case of a multiSig can detect upcoming changes and have the time to...

IndexPool.sol#_pow() Wrong implementation

Handle WatchPug Vulnerability details function powuint256 a, uint256 n internal pure returns uint256 output output = n % 2 != 0 ? a : BASE; for n /= 2; n != 0; n /= 2 a = a a; if n % 2 != 0 output = output a; 1. a a without div by BASE will accumulate decimals unexpectedly and leads to overflow...

onlyOwner Role Can Manipulate Price Oracle

Handle leastwood Vulnerability details Impact The V2 LendingController.sol contract incorporates a custom oracle used to fetch the relevant price feeds for token pairs. The onlyOwner role is controlled by a modified timelock contract callable only from a single externally owned account. This role...

Auction.sol#settleAuction() Mishandling bounty state could potentially disrupt settleAuction()

Handle WatchPug Vulnerability details function withdrawBountyuint256 memory bountyIds internal // withdraw bounties for uint256 i = 0; i bountyIds.length; i++ Bounty memory bounty = bountiesbountyIdsi; requirebounty.active; IERC20bounty.token.transfermsg.sender, bounty.amount; bounty.active =...

PostAuctionLauncher can be manipulated by a caller other than the owner

Handle tensors Vulnerability details Impact A comment in the code asks whether it is safe to allow anyone to call PostAuctionLauncher, finalize. In the case that an attacker can get even a few wei of the auction token, it is not safe. Suppose an attacker somehow gets a small amount of the auction...

Last person to withdraw his tokens might not be able to do this, in Crowdsale (edge case)

Handle gpersoon Vulnerability details Impact Suppose a Crowdsale is successful and enough commitments are made before the marketInfo.endTime. Suppose marketStatus.commitmentsTotal == marketInfo.totalTokens -1 // note this is an edge case, but can be constructed by an attacker Then the function...

Vault: Withdrawal amount isn't un-normalized

Handle hickuphh3 Vulnerability details Impact In withdraw, the withdrawal amount is the proportion of the normalized amounts of all the tokens in the vault and its strategies. However, this amount isn't un-normalized to the output token's decimals, thus leading to an erroneous token amount being...

The function addToken does not check if the token was already added

Handle hrkrshnn Vulnerability details addToken does not check if the token was already added The function addToken does not check if the token was already present. function addToken address vault, address token external override notHalted onlyStrategist requireallowedTokenstoken, "!allowedTokens"...

SushiToken transfers are broken due to wrong delegates accounting on transfers

Handle cmichel Vulnerability details When minting / transferring / burning tokens, the SushiToken.beforeTokenTransfer function is called and supposed to correctly shift the voting power due to the increase/decrease in tokens for the from and two accounts. However, it does not correctly do that, i...

harvestNextStrategy never executes because lastCalled is 0 and stays 0

Handle gpersoon Vulnerability details Impact The function harvestNextStrategy of Harvester.sol checks canHarvest to make sure it can harvest. Initially strategy.lastCalled will be 0 so canHarvest will return false. Thus the require in harvestNextStrategy fails And it never reaches the point where...

lack of validation for the v and s value in recover() funciton

Handle JMukesh Vulnerability details Impact due to lack of checking of v and s value in recover it become prone to signature malleability Proof of Concept check out the tryRecover of ECDSA.sol Tools Used manual reveiw Recommended Mitigation Steps add necessary check to make the signature unique -...

Allowance checks not correctly implemented

Handle cmichel Vulnerability details Vulnerability Details The nTokenAction implements two token approvals, the nTokenWhitelist which is always used first, and the nTokenAllowance which is checked second. If the nTokenWhitelist does not have enough allowance for the transfer, the transaction fail...

Market whitelist does not work

Handle cmichel Vulnerability details The RCTreasury.marketWhitelistCheck function gets the marketWhitelistmsgSender variable and performs a special check if it's non-zero. However, there's no way to set the whitelist in the first place making this function unnecessary. Impact The market whitelist...

OracleManagerEthVsBtc price data could be stale

Handle cmichel Vulnerability details There is no check in OracleManagerEthVsBtc.getLatestPrice 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 stale data."...

Incorrect balance computed in getUsersConfirmedButNotSettledSynthBalance()

Handle hack3r-0m Vulnerability details Consider the following state: longsynthbalace = 300; shortsynthbalace = 200; marketUpdateIndex1 = x; userNextPricecurrentUpdateIndex = 0; userNextPricesyntheticTokentoShiftAwayFrommarketSide1true = 0; batchedamountSyntheticTokentoShiftAwayFrommarketSide1true...

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

Possible DoS attack in creating new DAO proposals

Handle shw Vulnerability details Impact The functions of creating new DAO proposals e.g., newActionProposal are permissionless. Anyone can create a new proposal by paying some fees in SPARTA, as long as the previous proposal is closed. Thus, an attacker could then front-run proposals of benign...

ERC20 return values not checked

Handle cmichel Vulnerability details The ERC20.transfer and ERC20.transferFrom functions return a boolean value indicating success. This parameter needs to be checked for success. Some tokens do not revert if the transfer failed but return false instead. This is generally not an issue when the...

[PoolFactory.sol] createPoolADD() function is payable but does not contain a function to withdraw funds

Handle maplesyrup Vulnerability details Impact This is a medium risk vulnerability as it can affect funds within pools that are created via this contract. With no withdraw functions being implemented, it is possible that funds can be locked in the contract with no way to retrieve earnings or...

Synth: cannot change approval once set to max value

Handle cmichel Vulnerability details The Synth.approve function performs a no-op if the allowance is currently set to typeuint256.max. This leads to the issue that approvals cannot be changed anymore once they are set to the max value. Impact Imagine someone approving an operator with the max val...

Router.removeLiquiditySingle(uint256,bool,address) has unchecked transfers

Handle heiho1 Vulnerability details Impact Router.removeLiquiditySingleuint256,bool,address on lines 121, 126, 129 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 instead of...

DaoVault.withdraw(address,address) potentially subject to timestamp manipulation

Handle heiho1 Vulnerability details Impact DaoVault.withdrawaddress,address uses block.timestamp based comparisons can be affected by miner behavior, leading to withdrawal impacts on the user. Proof of Concept Tools Used Slither Recommended Mitigation Steps An external time oracle like ChainLink...

Broken access control leads to protocol functionality freeze

Handle 0xRajeev Vulnerability details Impact The contracts use an access control pattern where the contract deployer is included in the onlyDAO modifier which is used for authorized access to critical functions. Such contracts also include a purgeDeployer function which renounces sets to...

Missing slippage checks

Handle cmichel Vulnerability details The Router and Pool does not implement any slippage checks with comparing the swap / liquidity results with a minimum swap / liquidity value. Impact Users can be frontrun and receive a worse price than expected when they initially submitted the transaction...

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

The fallback receiver address could get twice the toSend amount

Handle s1m0 Vulnerability details In that block of code there are 2 external call inside a try/catch statements. In both the catch the toSend amount is transferred to the fallback receiver address effectively transferring twice if the 2 external call fail. Impact In the fulfill function the...

addFunds and execute may send tokens twice

Handle pauliax Vulnerability details Impact Both calls to IFulfillHelper addFunds and execute are wrapped in separate try/catch statements so basically if addFunds succeeds but execute fails or both of these functions fail, the catch will still send assets to the receivingAddress. I think these...

WETH not verified to be one of the pair tokens

Handle gpersoon Vulnerability details Impact The ETH functions of LendingPair don't check that WETH is one of the 2 pairs of the contract and introduces a third token. The third token ETH/WETH falls outside the other checks that the contract is doing. You could accidentally pay ETH/WETH to the...

User prepare can be denied

Handle cmichel Vulnerability details Vulnerability Details Upon observing a prepare transaction, an attacker can frontrun it with the same invariantData but an amount of a single wei. This inserts a value into variantTransactionDatadigest and the original transcation will fail because of the...

Exposure.sortVaultsByDelta can underflow

Handle cmichel Vulnerability details Vulnerability Details The sortVaultsByDelta function performs an unsafe subtraction on two uint256 before casting them to int256. The subtraction can underflow and the cast to int256 can either fail and revert the transaction if greater than typeint256.max, or...

A market's hourly average price can be biased by a large number of trades

Handle shw Vulnerability details Impact An attacker can artificially move a market's hourly average price i.e., the result of getHourlyAvgTracerPrice by executing a large number of trades on the market with only paying gas fees. Proof of Concept The hourly average price is calculated by the...

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

Unused return value from transfer()/transferFrom()

Handle JMukesh Vulnerability details Impact return value from transfer/transferFrom ensure success of the call and if not , it describe the reason for. It is usually good to add a require-statement that checks the return value or to use something like safeTransfer; unless one is sure the given...

Bots can frontrun market maker orders and cancel orders.

Handle tensors Vulnerability details Impact Market makers will not be able to have competitive spreads or cancel their orders to update their strategies. It will be difficult for them to provide liquidity on the platform, which will hurt the platform overall. Proof of Concept In traditional finan...

prb-math not audited

Handle gpersoon Vulnerability details Impact The library prb-math documents that it is not audited by a security researcher. This means its more risky to rely on this library. Proof of Concept // The contracts have not been audited by a security researcher. Tools Used Recommended Mitigation Steps...

IdleYieldSource doesn't use mantissa calculations

Handle tensors Vulnerability details Impact Because mantissa calculations are not used in this case to account for decimals, the arithmetic can zero out the number of shares or tokens that should be given. For example, say I deposit 1 token, expecting 1 share in return. On L95, if 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. -...

Missing call to removeOldBids may affect foreclosure

Handle 0xRajeev Vulnerability details Impact Orderbook.removeBids as commented “///remove bids in closed markets for a given user ///this can reduce the users bidRate and chance to foreclose” removeOldBids is performed currently in Market.newRental and Treasury.deposit to “do some cleaning up, it...

Critical uberOwner address changes should be a two-step process

Handle 0xRajeev Vulnerability details Impact As specified, uberOwners of Factory, Orderbook and Treasury have the highest privileges in the system because they can upgrade contracts of market, Nfthub, order book, treasury, token and factory which form the critical components of the protocol. The...

Missing checkOnERC721Received deviates from ERC721 and could lock/lose NFTs

Handle 0xRajeev Vulnerability details Impact ERC721 specification for safeTransferFrom says: “this function checks if to is a smart contract code size 0. If so, it calls onERC721Received on to and throws if the return value is not bytes4keccak256“onERC721Receivedaddress,address,uint256,bytes”.”...

erc20 transfer and transferFrom functions

Handle pauliax Vulnerability details Impact When transfering erc20 tokens, functions transfer and transferFrom are used. These functions return boolean to indicate if the action was successful, however, none of the usages check the returned value: erc20.transferFrommsgSender, addressthis, amount;...

Users can avoid paying borrowing interest after the fyToken matures

Handle shw Vulnerability details Impact According to the protocol design, users have to pay borrowing interest when repaying the debt with underlying tokens after maturity. However, a user can give his vault to Witch and then buy all his collateral using underlying tokens to avoid paying the...

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

ERC20ConvictionScore allows transfers to special TOTAL_GOVERNANCE_SCORE address

Handle cmichel Vulnerability details Vulnerability Details The credit score of the special addresstypeuint160.max is supposed to represent the sum of the credit scores of all users that are governors. But any user can directly transfer to this address increasing its balance and accumulating a...