BadgerYieldSource balanceOfToken share calculation seems wrong

Handle cmichel Vulnerability details When suppling to the BadgerYieldSource, some amount of badger is deposited to badgerSett and one receives badgerSett share tokens in return which are stored in the balances mapping of the user. So far this is correct. The balanceOfToken function should then...

YieldSourcePrizePool_canAwardExternal does not work

Handle cmichel Vulnerability details The idea of YieldSourcePrizePoolcanAwardExternal seems to be to disallow awarding the interest-bearing token of the yield source, like aTokens, cTokens, yTokens. "@dev Different yield sources will hold the deposits as another kind of token: such a Compound'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...

User deposits can be turned into sponsors and then be stolen

Handle cmichel Vulnerability details Vulnerability Details When a user deposits to the treasury they first approve the contract and then call its deposit action which performs an ERC20.transferFrom. It's possible for an attacker to frontrun the final deposit transaction after the user approval an...

Flows can bypass market and global pause

Handle 0xRajeev Vulnerability details Impact Ability to pause all token transfers and all state changes for contracts is a “guarded-launch” best-practice for emergency situations for newly launched projects. The project implements this using a marketsPaused flag per market and a globalPause flag...

Unchecked return value from ERC20

Handle cmichel Vulnerability details Vulnerability Details The ERC20.transfer and ERC20.transferFrom functions return a boolean value indicating success. This parameter needs to be checked for success. According to the standard the return value must be checked for true, otherwise the transfer wil...

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

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

Unchecked ERC20 transfers can cause lock up

Handle axic Vulnerability details Impact Some major tokens went live before ERC20 was finalised, resulting in a discrepancy whether the transfer functions a should return a boolean or b revert/fail on error. The current best practice is that they should revert, but return “true” on success...

User can redeem more tokens by artificially increasing the chi accrual

Handle shw Vulnerability details Impact A user can artificially increase the chi accrual after maturity by flash borrow on Compound, which affects the exchange rate used by the chi oracle. As a result, the user redeems more underlying tokens with the same amount of fyTokens since the accrual is...

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

auth only works well with external functions

Handle gpersoon Vulnerability details Impact The auth modifier of AccessControl.sol doesn't work as you would expect. It checks if you are authorized for "msg.sig", however msg.sig is the signature of the first function you have called, not of the current function. So if you call function A, whic...

Incorrect implementation of arctan in the contract FairSideFormula

Handle shw Vulnerability details Impact The current implementation of the arctan formula in the contract FairSideFormula is inconsistent with the referenced paper and could cause incorrect results when the input parameter is negative. The erroneous formula affects the function calculateDeltaOfFSD...

Incorrect type conversion in the contract ABC makes users unable to burn FSD tokens

Handle shw Vulnerability details Editing on a previous post to correct some details Impact The function calculateDeltaOfFSD of contract ABC incorrectly converts an int256 type parameter, reserveDelta, to uint256 by explicit conversion, which in general results in an extremely large number when th...

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

Call to swapExactTokensForETH in liquidateDai() will always fail

Handle 0xRajeev Vulnerability details Impact liquidateDai calls Uniswap’s swapExactTokensForETH to swap Dai to ETH. This will work if msg.sender, i.e. FSD contract, has already given the router an allowance of at least amount on the input token Dai. Given that there is no prior approval, the call...

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

Unchecked return value of transferFrom in function timeLockERC20

Handle shw Vulnerability details Impact In the function timeLockERC20 line 610, the return value of IERC20.transferFrom is unchecked. The return value could be false if the transferred token is not ERC20-compliant, indicating that the transfer fails. In that case, the variable timelockERC20Balanc...

Lack of address input validation will lock tokens in contract

Handle 0xRajeev Vulnerability details Impact Functions timeLockERC721 and timeLockERC20 are used by the vault owner to timelock tokens in the vault with a specified recipient address as the only one with the right to withdraw after timelock expiry. If a zero/incorrect recipient address is used he...

getRandomTokenIdFromFund yields wrong probabilities for ERC1155

Handle @cmichelio Vulnerability details Vulnerability Details NFTXVaultUpgradeable.getRandomTokenIdFromFund does not work with ERC1155 as it does not take the deposited quantity1155 into account. Impact Assume tokenId0 has a count of 100, and tokenId1 has a count of 1. Then getRandomId would have...

_sendForReceiver is vulnerable to reentrancy. This enables a receiver to drain the remaining fees to distribute.

Handle janbro Vulnerability details Summary sendForReceiver is vulnerable to reentrancy. This enables a receiver to drain the remaining fees to distribute. Risk Rating Critical Vulnerability Details NFTXFeeDistributor.sol Line 163: bool success, bytes memory returnData =...

This is a test submission and can be deleted

Handle adamavenir Vulnerability details Impact Detailed description of the impact of this finding. Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. Tools Used Recommended Mitigation Steps --- T...

Fee Distribution Re-Entrancy

Handle 0xsomeone Vulnerability details Impact The distribute function of NFTXFeeDistributor has no access control and will invoke a fallback on the fee receivers, meaning that a fee receiver can re-enter via this function to acquire their allocation repeatedly potentially draining the full balanc...

Use of transfer() may lead to failures

Handle 0xRajeev Vulnerability details Impact Although transfer and send have been recommended as a security best-practice to prevent reentrancy attacks because they only forward 2300 gas, the gas repricing of opcodes such as with EIP-2929 in the recent Berlin fork may break deployed contracts...

mint for 0 cost when the sale is over

Handle paulius.eth Vulnerability details Impact function getPrice returns 0 when elapsed saleDuration, it does not revert when the sale is over and function mint does not check that. So a 0 salePrice will be used to charge the msg.sender and make a useless transfer to the beneficiary. I am not su...

init functions can be frontrun

Handle @cmichelio Vulnerability details Vulnerability Details The init function that initializes important contract state can be called by anyone. Impact The attacker can initialize the contract before the legitimate deployer, hoping that the victim continues to use the same contract. In the best...

Unrestricted access to lockUnits allows an attacker to steal funds from any user.

Handle shw Vulnerability details Impact The lockUnits and unlockUnits functions in Pools.sol allow anyone to call without any restrictions or access control on the caller. An attacker can steal any user's member units by directly calling lockUnits. Proof of Concept Referenced code:...

Wrong slippage protection on Token -> Token trades

Handle @cmichelio Vulnerability details Vulnerability Details The Router.swapWithSynthsWithLimit allows trading token to token and specifying slippage protection. A token to token trade consists of two trades: 1. token to base 2. base to token The slippage protection of the second trade base to...

changeDAO should be a two-step process in Vader.sol

Handle 0xRajeev Vulnerability details Impact changeDAO updates DAO address in one-step. If an incorrect address is mistakenly used and voted upon then future administrative access or recovering from this mistake is prevented because onlyDAO modifier is used for changeDAO, which requires msg.sende...

Bypass or reduction on the lockup period.

Handle shw Vulnerability details Impact In Pool.sol, the lockup restriction of withdrawal can be bypassed or reduced if new liquidity providers cooperate with existing ones. Proof of Concept 1. A liquidity provider Alice deposits liquidity assets into the pool and gained some FDTs. She then waits...

Impossible to call withdrawReward fails due to run out of gas

Handle s1m0 Vulnerability details Impact The withdrawReward fails due to the loop at . From my testing the dayDiff would be 18724 and with a gasLimit of 9500000 it stops at iteration 270 due to the fact that lastUpdatedDay is not initialized so is 0. Other than that it could run out of gas also f...

Inconsistent usage of applyInterest

Email address [email protected] Handle paulius.eth Eth address 0x523B5b2Cc58A818667C22c862930B141f85d49DD Vulnerability details It is unclear if the function applyInterest is supposed to return a new balance with the interest applied or only the accrued interest? There are various usages of it,...

Missing checks if pairs equal tokens

Email address [email protected] Handle @cmichelio Eth address 0x6823636c2462cfdcD8d33fE53fBCD0EdbE2752ad Vulnerability details The UniswapStyleLib.getAmountsOut, PriceAware.setLiquidationPath and others don't check that path.length + 1 == tokens.length which should always hold true. Also, it does n...

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

PriceAware uses prices from getAmountsOut

Vulnerability details getPriceFromAMM relies on values returned from getAmountsOut which can be manipulated e.g. with the large capital or the help of flash loans. The impact is reduced with UPDATEMINPEGAMOUNT and UPDATEMAXPEGAMOUNT, however, it is not entirely eliminated. Impact Email address...

Attacker can call update_market frequently to halt the increase of secRewardsPerShare

Lines of code Vulnerability details Impact Attacker can call updatemarket frequently to halt the increase of secRewardsPerShare. Proof of Concept market.secRewardsPerShare += uint128blockDelta 1e18 / marketSupply; // TODO: Scaling Suppose marketSupply is 1e21, then the attacker can call...

Upgraded Q -> 3 from #146 [1706014097180]

Judge has assessed an item in Issue 146 as 3 risk. The relevant finding follows: L-6: OLAS minting via treasury is not guaranteed The OLAS.mint… method does not revert if the requested amount cannot be limited due to the inflation limit: function mintaddress account, uint256 amount external //...

Upgraded Q -> 2 from #530 [1705001094273]

Judge has assessed an item in Issue 530 as 2 risk. The relevant finding follows: QA-04 owner of AuctionHouse should not be allowed to change parameters for the ongoing auction --- The text was updated successfully, but these errors were encountered: All reactions...

Time delay for operations scheduled by the community multisig (CM) in timelock pose a risk to the protocol and it users

Lines of code Vulnerability details Impact Time sensitive operations done by the CM, like taking actions in case of an security exploit, are subject to the minDelay of the Timelock contract. Such operations are time sensitive and executing them several minutes later can result in significant loss...

claimOwnerIncentives x depositServiceDonationsETH x checkpoint x-entrancy Attack to get instant topups

Lines of code Vulnerability details Impact Double Topup via claimOwnerIncentives- depositServiceDonationsETH - checkpoint - claimOwnerIncentives loop Proof of Concept - Deploys Attacking Smart Contract - Executes 1st transaction via Attacking Contract to trigger depositServiceDonationsETH - Execu...

[M2] DrainServicesSlashedFunds has not check for received funds

Lines of code Vulnerability details Impact ​ Loss of funds. Analysis of the vulnerability The function to drain funds from Service Registry is not checking that is receiving the correct amount from ServiceRegistry. /// @dev Drains slashed funds from the service registry. /// @return amount Draine...

Withdraw amount returned by getLiquidityAmountsAndPositions may be incorrect

Lines of code Vulnerability details Impact The getLiquidityAmountsAndPositions function in the liquiditylockbox contract is used to calculate the liquidity amounts and positions to be withdrawn for a given total withdrawal amount. It iterates through each deposited position following a FIFO order...

[M1] LastDonationBlockNumber should be updated at the beginning of the function to prevent from reentracy attack

Lines of code Vulnerability details Impact The ineffectiveness of an update against a flashloans attack. Analysis of the vulnerability You update lastDonationBlockNumber after every donation. However, you update is done at the end of the function trackServiceDonations function trackServiceDonatio...

Silent failure in user reward transfer in Treasury.withdrawToAccount() can lead to loss of rewards

Lines of code Vulnerability details Impact The withdrawToAccount function of the Treasury contract is designed to send ETH rewards and OLAS top-ups to a specified account. However, there is a potential issue where a user's reward transfer could silently fail. This occurs when the...

Upgraded Q -> 2 from #286 [1704653766013]

Judge has assessed an item in Issue 286 as 2 risk. The relevant finding follows: L-02 Quorum for existing piece cannot be changed L-03 Token inflation gives advantage to new pieces --- The text was updated successfully, but these errors were encountered: All reactions...

repay(), liquidate() and liquidateWLp() receive shares as argument, which may revert if from approval to tx settled blocks have passed

Lines of code Vulnerability details Impact repay, liquidate and liquidateWLp transactions revert if users approve the exact repay amount they need in the frontend and only after some blocks have passed is the transaction settled. This happens because the interest accrual is by timestamp, so the...

In some pools, borrowers can maliciously prevent liquidatePosition()

Lines of code Vulnerability details Vulnerability details in liquidatePosition At the end of the liquidation, the liquidation fee will be transferred to the liquidator. function liquidatePosition DataStruct.ClosePositionParams calldata params, address borrower external override nonReentrant...

wrong implementation of maxHeap.extractMax();

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. wrong implementation of maxHeap.extractMax; as it should return values but here its not assigned to anything. Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots, log...

Some buyers wont get expected tokens minted due to precision loss

Lines of code Vulnerability details Impact The ERC20TokenEmitter.buyToken mints tokens according to the configured bps per address. This is due to the below code's implementation in buyToken function. for uint256 i = 0; i 0 // transfer tokens to address mintaddressesi, uint256totalTokensForBuyers...

Lack of receive() inside MoneyMarketHook contract when interacting with WNATIVE.

Lines of code Vulnerability details Impact MoneyMarketHook is expected to work with the native balance when interacting with WNATIVE, especially when params.returnNative is set to true during the execute operation. However, due to the lack of a receive function inside the contract, MoneyMarketHoo...