Access restrictions on NotionalV1ToNotionalV2.notionalCallback can be bypassed

Handle cmichel Vulnerability details Vulnerability Details The NotionalV1ToNotionalV2.notionalCallback is supposed to only be called from the verified contract that calls this callback but the access restrictions can be circumvented by simply providing sender = this as sender is a parameter of th...

Freeze The Bridge Via Large ERC20 Names/Symbols/Denoms

Handle nascent Vulnerability details Ethereum Oracles watch for events on the Gravity.sol contract on the Ethereum blockchain. This is performed in the checkforevents function, ran in the ethoraclemainloop. In this function, there is the following code snippet: let erc20deployed = web3...

Possible reentrancy in balanceOf, decimals, mint

Handle tensors Vulnerability details Impact Registering tokens that aren't properly vetted can lead to a loss of funds if the token has callbacks. CREAM finance got hacked in a similar way because the ampleforth token had a callback in the transfer method that wasn't noticed when they vetted it...

findNewOwner edgecase

Handle gpersoon Vulnerability details Impact In the function findNewOwner of RCOrderbook, as loop is done which included the check loopCounter maxDeletions Afterwards a check is done for "loopCounter != maxDeletions" to determine if the processing is finished. If loopCounter == maxDeletions then...

Exchange rates from Compound are assumed with 18 decimals

Handle shw Vulnerability details Impact The CTokenMultiOracle contract assumes the exchange rates borrowing rate of Compound always have 18 decimals, while, however, which is not true. According to the Compound documentation, the exchange rate returned from the exchangeRateCurrent function is...

TimeLock cannot schedule the same calls multiple times

Handle cmichel Vulnerability details The TimeLock.schedule function reverts if the same targets and data fields are used as the txHash will be the same. This means one cannot schedule the same transactions multiple times. Impact Imagine the delay is set to 30 days, but a contractor needs to be pa...

No ERC20 safe* versions called

Handle cmichel Vulnerability details The claim function performs an ERC20 transfer rewardsToken.transferto, claiming; but does not check the return value, nor does it work with all legacy tokens. Some tokens like USDT don't correctly implement the EIP20 standard and their transfer/transferFrom...

Missing events/timelocks for owner/admin only functions that change critical parameters

Handle 0xRajeev Vulnerability details Impact Owner/admin only functions that change critical parameters should emit events and have timelocks. Events allow capturing the changed parameters so that off-chain tools/interfaces can register such changes with timelocks that allow users to evaluate the...

OracleManagerFlippening_V0 wrong decimals

Handle cmichel Vulnerability details The OracleManagerFlippeningV0.updatePrice function states that it wants to return the eth dominance as a percentage where 100% = 1e20. It's unclear why there is a division by 1e10 to compute the bitcoin market cap: uint256btcPrice btcSupply 1e10 Impact The pri...

Incorrect use of latestMarket instead of marketIndex in several functions of LongShort

Handle shw Vulnerability details Impact Some part of the logic in the initializeMarket and seedMarketInitially functions of LongShort incorrectly operates on the latestMarket instead of marketIndex, the provided parameter. Since the latestMarket is not necessary to be the market to be initialized...

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

Staker.sol: Wrong values returned in edge cases of _calculateFloatPerSecond()

Handle hickuphh3 Vulnerability details Impact In calculateFloatPerSecond, the edge cases where full rewards go to either the long or short token returns return 1e18 k longPrice, 0; and return 0, 1e18 k shortPrice; respectively. This is however 1e18 times too large. We can verify this by checking...

Usage of safeApprove

Handle pauliax Vulnerability details Impact function approveMax uses safeApprove. This function only works if the current approval is 0. Consider clearing previous approval safeApprove0 before setting the max value again. The same issue can happen with SwappableYieldSource if, for example, source...

Inconsistent balance when supplying transfer-on-fee or deflationary tokens

Handle shw Vulnerability details Impact The supplyTokenTo function of SwappableYieldSource assumes that amount of depositToken is transferred to itself after calling the safeTransferFrom function and thus it supplies amount of token to the yield source. However, this may not be true if the...

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

[Bug] A critical bug in bps function

Handle hrkrshnn Vulnerability details A critical bug in bps function: PoolBase.sol function bps internal pure returns IERC20 rt // These fields are not accessible from assembly bytes memory array = msg.data; uint256 index = msg.data.length; // solhint-disable-next-line no-inline-assembly assembly...

Incorrect internal balance bookkeeping

Handle walker Vulnerability details type: Incorrect Assumptions on External Systems The sherlock smart contract system uses internal bookkeeping of arbitrary ERC20 token balances. It doesn't assert that the ERC20 doesn't implement some non-standard behaviour. For example, deflationary tokens, or...

Flash loan manipulation on getPoolShareWeight of Utils

Handle shw Vulnerability details Impact The getPoolShareWeight function returns a user's pool share weight by calculating how many SPARTAN the user's LP tokens account for. However, this approach is vulnerable to flash loan manipulation since an attacker can swap a large number of TOKEN to SPARTA...

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

Improper access control of claimAllForMember allows anyone to reduce the weight of a member

Handle shw Vulnerability details Impact The claimAllForMember function of Dao is permissionless, allowing anyone to claim the unlocked bonded LP tokens for any member. However, claiming a member's LP tokens could decrease the member's weight in the BondVault, thus affecting the member's votes and...

Deployer backdoors in DAOVault, Router and SynthVault contracts

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

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

Dao.calcReward(address) has potential division before multiplication arithmetic errors

Handle heiho1 Vulnerability details Impact Dao.calcRewardaddress on lines 203 and 204 are potentially problematic in that division may potentially truncate values resulting in loss of precision. Proof of Concept Tools Used Slither Recommended Mitigation Steps Potentially this issue is lessened wi...

Unused/Incorrect onlyDAO modifier could be an indication of missing access control

Handle 0xRajeev Vulnerability details Impact Unused/incorrect access control modifier is typically an indication of missing critical authorization checks. The onlyDAO modifier used in various protocol contracts is present in Synth.sol but unused in any of the contract functions and is also...

Owner can burn other users token shares

Handle JMukesh Vulnerability details Impact This privilege should not be given to the owner, which can burn other users token shares, due to this user will not be able to claim their liquidity. Due to this privilege user will lost control over liquidity amount Proof of Concept Tools Used manual...

Use of deprecated Chainlink API

Handle 0xRajeev Vulnerability details Impact UniswapV3Oracle contract uses Chainlink’s deprecated API latestAnswer. Such functions might suddenly stop working if Chainlink stopped supporting deprecated APIs. Impact: Deprecated API stops working. Prices cannot be obtained. Protocol stops and...

Total LP supply & total debt accrual is wrong

Handle cmichel Vulnerability details The total debt and total supply only increase when debt/supply is minted to the user when it should increase by the entire new interest amount on each accrual. function accrueAccountaddress account public distributeRewardaccount; // accrue only updates...

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

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

Expired transfers will lock user funds on the sending chain

Handle 0xRajeev Vulnerability details Impact The cancelling relayer is being paid in receivingAssetId on the sendingChain instead of in sendingAssetID. If the user relies on a relayer to cancel transactions and that receivingAssetId asset does not exist on the sending chain assuming only...

addLiquidity can be denied

Handle cmichel Vulnerability details Vulnerability Details The addLiquidity function can be called by anyone to transfer funds from the router address specified as a function argument. These funds must be approved first by the router prior to calling this function. There are different griefing...

Funds can be stolen because of approval + send

Handle cmichel Vulnerability details Vulnerability Details The fulfill transaction on the receiving chain first approves the txData.callTo contract with the toSend amount. It then tries to call the addFunds and execute actions on txData.callTo. When any of the calls reverts, the funds are sent to...

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

Use latestRoundData instead of latestAnswer

Handle adelamo Vulnerability details Impact Use latestRoundData instead of latestAnswer for chainlink Oracles in order to be able to run more validations like roundId, rawPrice, , updateTime, answeredInRound = AggregatorV3Interfacesource.source.latestRoundData; requirerawPrice 0, "Chainlink price...

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

Lack of input validation on onlyOwner critical parameters

Handle 0xRajeev Vulnerability details Impact The owner potentially untrustworthy/malicious of the prize pool is allowed to set a liquidation cap for guarded launch and the credit rate and limit parameters which affect the crucial fairness of the pool. However, there is no input validation on thes...

Deposits can be denied by abusing maxContractBalance

Handle cmichel Vulnerability details Vulnerability Details The treasury implements a max contract balance check in the deposit function: require erc20.balanceOfaddressthis + amount This is not only restricted to whales, miners/users can do the same using same-block cross-transaction flashloans an...

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

Market-specific pause is not checked for sponsor

Handle cmichel Vulnerability details Vulnerability Details The treasury only checks its globalPause field but does not check its market-specific marketPaused field for Treasury.sponsor. A paused market contract can therefore still deposit as a sponsor using Market.sponsor Impact The market-specif...

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

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

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

Potential griefing with DoS by front-running vault creation with same vaultID

Handle 0xRajeev Vulnerability details Impact The vaultID for a new vault being built is required to be specified by the user building a vault via the build function instead of being assigned by the Cauldron/protocol. An attacker can observe a build as part of a batch transaction in the mempool,...

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

Witch can't give back vault after 2x grab

Handle gpersoon Vulnerability details Impact The witch.sol contract gets access to a vault via the grab function, in case of liquidation. If the witch.sol contract can't sell the debt within a certain amount of time, a second grab can occur. After the second grab, the information of the original...

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

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

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