Witch lock vault waiting for better price

Handle adelamo Vulnerability details Impact On the Witch, you can grab any under collateralized vault for X amount of time without requiring any payment of collateral. function grabbytes12 vaultId public DataTypes.Vault memory vault = cauldron.vaultsvaultId; vaultOwnersvaultId = vault.owner;...

FYTokens can be minted for free

Handle cmichel Vulnerability details The core issue is that one can force the protocol to do an arbitrary trade in the pool using Ladle.roll. The function allows specifying a base amount and the protocol will mint as many fyTokens as needed for the trade, and trade them in the pool. This can be...

Duplication of Balance

Handle 0xsomeone Vulnerability details Impact It is possible to duplicate currently held ink or art within a Cauldron, thereby breaking the contract's accounting system minting units out of thin air. Proof of Concept The stir function of the Cauldron, which can be invoked via a Ladle operation,...

Flashloan griefing attack

Handle cmichel Vulnerability details Funds from contracts that approved a join and implement the flashloan interface can be stolen. One can call Join.flashLoanvulnerablecontract, token, amount and the contract's balance will be decreased by the fees they have to pay for the flashloan. One can...

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

Undercollateralized vaults' owner can be overwritten

Handle cmichel Vulnerability details The witch can Witch.grab vaults and the vaultOwnersvaultId field is set to the original owner. However, when the auction time is over and the debt has not been fully paid back, the original owner is not restored, and the witch can grab the same vault again,...

Contract Factory Replace

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

Uniswap Oracle uses wrong prices

Handle cmichel Vulnerability details The Uniswap oracle uses a mock contract with hard-coded prices to retrieve the price which is not feasible in production. Not sure if this is part of the contest, this will probably still be changed? But note that even when using the "real deal"...

Vaults are in liquidation forever instead of just for auction length

Handle cmichel Vulnerability details The witch can Witch.grab vaults and the vaultOwnersvaultId field is set to the original owner. The original vault owner is only restored if all debt balances.art is repaid by the liquidation engine. if balances.art - art == 0 // If there is no debt left, retur...

Unauthorized functions in Ladle.sol and PoolRouter.sol

Handle gpersoon Vulnerability details Impact Both Ladle.sol and PoolRouter.sol contain a function batch, which gives access to several internal functions. Some of those functions call functions in other contracts which have an "auth" access control mechanism. However several internal functions ca...

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

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

YieldMath.sol / Log2: >= or > ?

Handle gpersoon Vulnerability details Impact The V1 version of YieldMath.sol contains "=" larger or equal, while the V2 version of YieldMath.sol containt "" larger in the log2 function. This change doesn't seem logical and might lead to miss calculations. The difference is present in a number of...

auth collision possible

Handle gpersoon Vulnerability details Impact The auth mechanism of AccessControl.sol uses function selectors msg.sig as a unique role definition. Also the moduleCall allows the code to be extended. Suppose an attacker wants to add the innocent looking function "leftbranchblockuint32" in an new...

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

Users are charged twice the FDT tokens when tokenizing their convictions

Handle shw Vulnerability details Impact Users have to pay twice the FSD tokens when tokenizing their convictions if the locked variable is non-zero. Proof of Concept The first payment is made in the function tokenizeConviction of the contract ERC20ConvictionScore line 282, where a user transfer...

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

Handle shw Vulnerability 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 the provided parameter is negative. The extremely lar...

Should check return data from Chainlink aggregators

Handle shw Vulnerability details Impact The getEtherPrice function in the contract FSDNetwork fetches the ETH price from a Chainlink aggregator using the latestRoundData function. However, there are no checks on roundID nor timeStamp, resulting in stale prices. Proof of Concept Referenced code:...

Flash minting and burning can reduce the paid fees when purchasing a membership or opening a cost share request

Handle shw Vulnerability details Impact Users can pay fewer FSD tokens when purchasing a membership or opening a cost share request by flash minting and burning FSD tokens, which could significantly affect the FSD spot price. Proof of Concept The function getFSDPrice returns the current FSD price...

Wrong function called in addRegistrationTributeGovernance

Handle shw Vulnerability details Impact The addRegistrationTributeGovernance function in the contract FSD includes an incorrect function, addTribute. According to its function name, the called function should be addGovernanceTribute instead. Proof of Concept Referenced code: FSD.solL140 Recommend...

pendingWithdrawals not decreased after a withdraw

Handle shw Vulnerability details Impact The variable pendingWithdrawals in the contract Withdrawable is not decreased after the function withdraw is called, which causes the return value of function getReserveBalance less than it should be. This bug could cause incorrect results in several critic...

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

Conviction scoring fails to initialize and bootstrap

Handle 0xRajeev Vulnerability details Impact Conviction scores for new addresses/users fail to initialize+bootstrap in ERC20ConvictionScore’s updateConvictionScore because a new user’s numCheckpoints will be zero and never gets initialized. This effectively means that FairSide conviction scoring...

_calculateDeltaOfFSD fails when called with negative _reserveDelta

Handle cmichel Vulnerability details Vulnerability Details When reserveDelta is negative in ABC.calculateDeltaOfFSD the following branch is executed: if reserveDelta 0 uint256 capitalPostWithdrawal = capitalPool.subuint256reserveDelta; The type cast to uint256 is purely a reinterpretation of the...

TributeAccrual missing out-of-bounds checks

Handle cmichel Vulnerability details Vulnerability Details The addTribute and addGovernanceTribute functions underflow when there are no tributes: Tribute storage lastTribute = tributestotalTributes - 1 = tributes-1; // underflow Impact It's bad practice and the iteration with the offset in...

Locked funds are debited twice from user during tokenization leading to fund loss

Handle 0xRajeev Vulnerability details Impact During tokenization of conviction scores, the user can optionally provide FSDs to be locked to let it continue conviction accrual. However, the amount of FSDs specified for locking are debited twice from the user leading to fund loss for user. This, in...

ERC20ConvictionScore._updateConvictionScore uses stale credit score for governanceDelta

Handle cmichel Vulnerability details Vulnerability Details In ERC20ConvictionScore.updateConvictionScore, when the user does not fulfill the governance criteria anymore, the governanceDelta is the old conviction score of the previous block. isGovernanceuser = false; governanceDelta =...

Conviction score is not updated during tokenization if funds are locked

Handle 0xRajeev Vulnerability details Impact The updateConvictionScore on Line284 of tokenizeConviction is only called if user specifies zero locked funds. This leads to loss of accounting of user’s conviction score for tokenization since the last update for user if non-zero amount of FSDs are...

Conviction totals not updated during tokenization

Handle 0xRajeev Vulnerability details Impact updateConvictionScore function returns convictionDelta and governanceDelta which need to be used immediately in a call to updateConvictionTotalsconvictionDelta, governanceDelta for updating the conviction totals of conviction and governance-enabled...

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

setConvictionless can be front-run to prevent conviction reset

Handle 0xRajeev Vulnerability details Impact The denylist convictionless is meant to deny conviction scores for certain users and is set by the privileged roles timelock/FSD-owner in setConvictionless. The documentation says: “adjust which addresses are meant to not accrue a conviction score. 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...

Underflow in FSD.burn breaks withdrawals and could allow stealing reserve

Handle cmichel Vulnerability details Vulnerability Details The FSD.burn function performs the following computation to determine the tokenAmount that a user must own which is then burned function burnuint256 capitalDesired, uint256 tokenMaximum external uint256 tokenAmount =...

pendingWithdrawals just increments

Handle adelamo Vulnerability details Impact In Withdrawable.sol, every time a user wants to withdraw, the following code will get executed: function increaseWithdrawaladdress user, uint256 amount internal availableWithdrawaluser = availableWithdrawaluser.addamount; pendingWithdrawals =...

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

ERC20ConvictionScore.tokenizeConviction transfers locked balance from user twice

Handle cmichel Vulnerability details Vulnerability Details In tokenizeConviction when locked 0 the amount is first transferred from the user using an internal call to transfermsg.sender, addressfairSideConviction, locked;. It is then transferred a second time from the user in the...

ERC20ConvictionScore.tokenizeConviction does not update total conviction & governance score

Handle cmichel Vulnerability details Vulnerability Details In tokenizeConviction, when locked == 0 the updateConvictionScoremsg.sender, 0 function is called to update the user's conviction, however the delta is not added to the total credit / governance score. Impact The TOTALCONVICTIONSCORE and...

NFTs can never be redeemed back to their conviction scores leading to lock/loss of funds

Handle 0xRajeev Vulnerability details Impact Besides the conviction scores of users, there appears to be tracking of the FairSide protocol’s tokenized conviction score as a whole using fscAddress = addressfairSideConviction. This is evident in the attempted reduction of the protocol’s score when ...

Bug inside ABDKMathQuad library

Handle adelamo Vulnerability details Impact FairSideFormula library is using ABDKMathQuad library underneath. According to the ABDKMathQuad README, the range of values is the following: The minimum strictly positive subnormal value is 2^−16494 ≈ 10^−4965 and has a precision of only one bit. The...

Withdrawable.withdraw does not decrease pendingWithdrawals

Handle cmichel Vulnerability details Vulnerability Details The name pendingWithdrawals indicates that this storage variable tracks the withdrawals that need yet to be paid out which also matches the behavior in increaseWithdrawal. So it should be decreased when withdrawing in withdraw but it is...

ERC20ConvictionScore's governanceDelta should be subtracted when user is not a governor anymore

Handle cmichel Vulnerability details Vulnerability Details The TOTALGOVERNANCESCORE is supposed to track the sum of the credit scores of all governors. In ERC20ConvictionScore.updateConvictionScore, when the user does not fulfill the governance criteria anymore and is therefore removed, the...

Locked funds from tokenization are credited twice to user leading to protocol fund loss

Handle 0xRajeev Vulnerability details Impact The tokens optionally locked during tokenization are released twice on acquiring conviction back from a NFT. The incorrect double debit of locked funds during tokenization has been filed as a separate finding because it is not necessarily related and...

Eth may get stuck in contract

Handle JMukesh Vulnerability details Impact Istanbul hardfork increases the gas cost of the SLOAD operation and therefore breaks some existing smart contracts. In file withdrawable.sol, contract uses transfer to send eth from contract to EOA due which eth can get stuck. reason behind this is, aft...

Incorrect use of _addTribute instead of _addGovernanceTribute

Handle 0xRajeev Vulnerability details Impact The addRegistrationTributeGovernance function is called by the FSD network to update tribute when 7.5% is contributed towards governance as part of purchaseMembership. However, this function incorrectly calls addTribute as done in addRegistrationTribut...

ERC20ConvictionScore.acquireConviction implements wrong governance checks

Handle cmichel Vulnerability details Vulnerability Details There are two issues with the governance checks when acquiring them from an NFT: Missing balance check The governance checks in updateConvictionScore are: !isGovernanceuser && userConvictionScore = governanceThreshold && balanceOfuser =...

The function liquidateDai() fails

Handle s1m0 Vulnerability details Impact liquidateDai try to swap DAI to ETH through uniswap using swapExactTokenForETH. Before calling swapExactTokenForETH the smart contract should approve uniswap router to use amount of DAI. Also as for documentation If the to address is a smart contract, it...

Changing ERC20ConvictionScore.governanceThreshold leads to temporarily broken state

Handle cmichel Vulnerability details Vulnerability Details Changing the governanceThreshold breaks the governance credit score accounting as users who currently qualify for being a governor may not qualify anymore and this influences the quorum threshold. It can be changed using...

withdraw() does not decrease pendingWithdrawals

Handle pauliax Vulnerability details Impact contract Withdrawable function withdraw does not subtract from pendingWithdrawals thus it only increases and could make function getReserveBalance revert when the balance pendingWithdrawals. Recommended Mitigation Steps Add this line to withdraw:...

function addRegistrationTributeGovernance should call _addGovernanceTribute not _addTribute.

Handle pauliax Vulnerability details Impact function addRegistrationTributeGovernance mistakenly calls addTribute. FunctionaddGovernanceTribute is never called thus governance tributes are never set nor updated. Recommended Mitigation Steps Replace 'addTribute' with 'addGovernanceTribute' in...