During stake or deposit, users would not be rewared the correct Concur token, when MasterChef has under-supply of it.

Lines of code Vulnerability details Impact During stake or deposit, users would not be transferred the correct Concur token, when MasterChef has under-supply of it. There is an assumption that MasterChef contract would own enough Concur tokens so as to distribute to users as reward, during deposi...

Block miscalculations

Lines of code Vulnerability details Impact The updatePool function in MasterChef is supposed to account for all blocks before endBlock. But the if statement checks if the current block is after the deadline. Thus blocks between pool.lastRewardBlock and endBlock won't be accounted for iff...

Shelter sets wrong claimed field

Lines of code Vulnerability details The Sheler.withdraw function sets the claimedtokenuser field but uses the shares of msg.sender. An attacker can withdraw tokens several times passing different to addresses, each time the msg.sender's shares will be used to receive tokens. function withdrawIERC...

Owner can lock tokens in MasterChef

Lines of code Vulnerability details Impact Owner can remove a depositor. Since only depositors can deposit and withdraw, the owner may add a contract to the whitelist, let users deposit in the contarct and remove the depositor from the whitelist. Depositor's reward cannot be withdrawn then. And...

Owner can steal Concur rewards

Lines of code Vulnerability details Impact Owner can steal Concur rewards by adding a depositor and inflating other depositors' assigned balance of the token within the contract. Thus, the owner-managed depositor can get most all but one wei of the created tokens. Tools Used Manual analysis...

[WP-H14] ConvexStakingWrapper, StakingRewards Wrong implementation will send concur rewards to the wrong receiver

Lines of code Vulnerability details UserInfo storage user = userInfopidmsgSender; updatePoolpid; ifuser.amount 0 uint pending = user.amount pool.accConcurPerShare / concurShareMultiplier - user.rewardDebt; if pending 0 safeConcurTransferrecipient, pending; ConvexStakingWrapper, StakingRewards is...

USDMPegRecovery does not account for fees

Lines of code Vulnerability details Curve.fi pools charge fees when adding or removing liquidity. The only time fees are not applied are when withdrawals are done using removeliquidity. USDMPegRecovery keeps track of tokens deposited and withdrawn, but does not keep track of fees assessed on thes...

Repeated withdrawals from Shelter

Lines of code Vulnerability details Impact function withdraw in Shelter sets claimed flag: claimedtokento = true; but it never actually checks if the user has already claimed, so users can invoke the withdrawal function multiple times and claim more rewards than were entitled. Recommended...

Wrong reward token calculation in MasterChef contract

Lines of code Vulnerability details Impact When adding new token pool for staking in MasterChef contract function addaddress token, uint allocationPoints, uint16 depositFee, uint startBlock All other, already added, pools should be updated but currently they are not. Instead, only totalPoints is...

Remaining reward balance is wrongly updated

Lines of code Vulnerability details The ConvexStakingWrapper.calcRewardIntegral function makes the dreward = IERC20reward.token.balanceOfaddressthis; - reward.remaining amount available for claiming. Then it updates the reward.remaining value to the balance before the distribution. RewardType...

All Tokens Can Be Stolen From Shelter Contract

Lines of code Vulnerability details Impact function withdrawIERC20 token, address to external override requireactivatedtoken != 0 && activatedtoken + GRACEPERIOD block.timestamp, "shelter not activated"; uint256 amount = savedTokenstoken client.shareOftoken, msg.sender / client.totalSharetoken;...

Deposits after the grace period should not be allowed

Lines of code Vulnerability details Impact Function donate in Shelter shouldn't allow new deposits after the grace period ends, when the claim period begins. Otherwise, it will be possible to increase savedTokenstoken, and thus new user claim amounts will increase after some users might already...

deposit wrong implementation

Lines of code Vulnerability details the user is the user of msg.sender, which is always the StakingRewards contract, instead of recepient. one of the problems that are caused by this is that anyone who deposits, will get the rewards of all the other users, because everyone shares the same user...

[WP-H13] MasterChef.sol Users won't be able to receive the concur rewards

Lines of code Vulnerability details According to: README Implementation of deposit: /contracts/MasterChef.solL157-L180 MasterChef is only recording the deposited amount in the states, it's not actually holding the depositToken. depositToken won't be transferred from msgSender to the MasterChef...

[WP-H8] ConvexStakingWrapper.sol#_calcRewardIntegral Wrong implementation can disrupt rewards calculation and distribution

Lines of code Vulnerability details uint256 bal = IERC20reward.token.balanceOfaddressthis; uint256 dreward = bal - reward.remaining; // send 20 % of cvx / crv reward to treasury if reward.token == cvx || reward.token == crv IERC20reward.token.transfertreasury, dreward / 5; dreward = dreward 4 / 5...

Imbalance and LP gain is forever stuck in USDMPegRecovery

Lines of code Vulnerability details Impact After depositing into Curve, underlying USDM and pool3 of each LP share will change due to 1. Swap fee 2. Pool imbalance However, userLiquidity does not update according to current underlying LP balance. When guardian remove liquidity from Curve, at leas...

Potential Re-entrancy Attack via ETH or ERC777 Token Transfer

Lines of code Vulnerability details Impact The CEI pattern is not being implemented properly in the claimRewards function of the ConcurRewardPool.sol. function claimRewardsaddress calldata tokens external override for uint256 i = 0; i tokens.length; i++ uint256 getting = rewardmsg.sendertokensi;...

Unconstrained fee

Lines of code Vulnerability details Impact Token fee in MasterChef can be set to more than 100%, for example by accident causing all deposit calls to fail due to underflow on subtraction when reward is lowered by the fee, thus breaking essential mechanics. Note that after the fee has been set to...

Reentrancy in ConcurRewardPool::claimRewards

Lines of code Vulnerability details Impact Any address that has nonzero reward for a token tokensi is able to drain all contact token funds if the transfer function is reentrant for example, ERC777 token. As tokensi is an arbitrarily implemented, a reentrant transfer function can be assumed to be...

missing check on claimed token

Lines of code Vulnerability details Impact the withdraw can be used by user to claim a token, however this function didnt check of the user had already claimed or not, therefore setting claimedtokento = true; is useless, therefore the user can claimed in multiple times, till the contract is...

Potential for reentrancy on USDMPegRecovery.sol:withdraw()

Lines of code Vulnerability details Impact Re-entrancy Proof of Concept File: USDMPegRecovery.sol 110: function withdrawLiquidity calldata withdrawal external 111: Liquidity memory total = totalLiquidity; 112: Liquidity memory user = userLiquiditymsg.sender; 113: ifwithdrawal.usdm 0 114:...

Repeated Calls to Shelter.withdraw Can Drain All Funds in Shelter

Lines of code Vulnerability details Impact tl;dr Anyone who can call withdraw to withdraw their own funds can call it repeatedly to withdraw the funds of others. withdraw should only succeed if the user hasn't withdrawn the token already. The shelter can be used for users to withdraw funds in the...

Shelter funds can be stolen

Lines of code Vulnerability details Impact Shelter has a function withdraw that lets whitelisted users withdraw a specified amount of some token. The function does not check if the user has already withdrew the tokens. Since, a user can withdraw allowed amount any number of times, stealing all th...

ConvexStakingWrapper._calcRewardIntegral() Has An Accounting Error When Updating reward.remaining

Lines of code Vulnerability details Impact The ConvexStakingWrapper.sol implementation makes several modifications to the original design. One of the key changes is the way rewards are distributed to stakers. A new ConcurRewardPool.sol contract is used to store rewards, allowing users to claim...

ConvexStakingWrapper deposits and withdraws will frequently be disabled if a token that doesn't allow zero value transfers will be added as a reward one

Lines of code Vulnerability details Impact If deposits and withdraws are done frequently enough, the reward update operation they invoke will deal mostly with the case when there is nothing to add yet, i.e. reward.remaining match the reward token balance. If reward token doesn't allow for zero...

MasterChef.add must update pools first

Lines of code Vulnerability details The MasterChef.add function changes the total pool allocation but does not update other pools first. When other pools are finally updated at some point, then accConcurPerShare will be wrongly computed with their smaller allocPoint / newTotalAllocPoint share eve...

claimed of _to set to true

Lines of code Vulnerability details Impact Function withdraw in Shelter calculates the amount based on shares of msg.sender, but sets the claimed flag of to: uint256 amount = savedTokenstoken client.shareOftoken, msg.sender / client.totalSharetoken; claimedtokento = true; This means a malicious...

Shelter.claim does not check if already claimed

Lines of code Vulnerability details The Sheler.withdraw function sets the claimedtokenuser field but does not check if the user is allowed to claim by checking require!claimedtokenuser, "already claimed". function withdrawIERC20 token, address to external override requireactivatedtoken != 0 &&...

Same reward token in pools can break accounting

Lines of code Vulnerability details The ConvexStakingWrapper contract uses several reward pool tokens rewardspidindex.token and it can be that the same token is used for different pids. Indeed, the CVX/CRV tokens are always at index 0 and 1. The rewards will be distributed to the first pool id pi...

Shelter withdraw does not check if user already claimed

Lines of code Vulnerability details Impact Shelter withdraw does not check if user already claimed. This allow any user with non-zero claim to drain the Shelter. Proof of Concept function withdrawIERC20 token, address to external override requireactivatedtoken != 0 && activatedtoken + GRACEPERIOD...

unsafe cast can lead to theft

Lines of code Vulnerability details in the first link I provided, a hacker can call withdraw with amount = 2196, the amount of shares that will be burned is uint1922192 == 0. if the system has enough money, the hacker can steal 2192 tokens and pay nothing. for the second link, a user that provide...

safeConcurTransfer can cause loss of funds

Lines of code Vulnerability details Impact All calculations are rounded down, since a lack of tokens in the contracts cannot be rounding errors' fault. So the function is redundant. On the other hand, if the contract is undersupplied with Concur tokens, this will cause depositors to be sent less...

double spend in Shelter

Lines of code Vulnerability details in the withdraw function in Shelter, one can withdraw infinite times, and by doing so, draining the system. the function doesn't check that the shares aren't already withdrawn. --- The text was updated successfully, but these errors were encountered: All reacti...

[WP-H3] saleRecipient can rug buyers

Lines of code Vulnerability details In TokenSaleUpgradeable.solbuy, tokenIn will be transferred from the buyer directly to the saleRecipient without requiring/locking/releasing the correspoining amount of tokenOut. This allows the saleRecipient to rug the users simply by not transferring tokenOut...

Owner could frontrun with higher price

Lines of code Vulnerability details Impact User can buy tokenOut at price X and be front-run by the owner by setting a higher price. Proof of Concept Recommended Mitigation Steps Changes in price by the owner should have some time lock to protect the buyer from frontrunning --- The text was updat...

Users should be allowed to control accepted tokenOutPrice

Lines of code Vulnerability details Impact Users should be able to control the accepted price. The owner can anytime invoke function setTokenOutPrice and thus change the ratio of token in/out. Users have to trust the owner not to front-run them and make the tokens more expensive. Recommended...

Improper Upper Bound Definition On The OnlyOwner Function Variables

Lines of code Vulnerability details Impact During the documentation of the contest, It has been seen that the following comment added. Specific care should be put in: Economic exploits Rug Vectors However, onlyOwner function does not have upper bound definition on the related variables. Values th...

tokenOut is only required on finalize

Lines of code Vulnerability details Impact I thought of a potential rug pull from the owner: when users buy tokenOut, it is not required that the contract has already escrowed enough tokenOut. It is only required when finalizing the sale: require tokenOut.balanceOfaddressthis = totalTokenOutBough...

Option to have setTokenOutPrice, will impact Token sale and user behavior

Lines of code Vulnerability details There is no loss of funds, but there is a possibility that the current code will dictate user behavior which is not intended. Hence setting the risk rating as medium. Impact There is an option to setTokenOutPrice by the owner. There can be two cases. case-1. th...

Sale operator may frontrun purchases in order to modify the price arbitrarily

Lines of code Vulnerability details Impact Buyer's cannot set a minimum amount of CTDL that they will expect so BadgerDAO can force them to receive a smaller amount than expected. Proof of Concept The buy function only allows the buyer to specify an amount of tokenIn to send to the contract and...

Possible rug #3

Lines of code Vulnerability details Impact Tokens can be consumed by the contract and sent to the saleRecipient address without any guarantee that sale tokens will ever be deposited in the sale contract. The sold tokens are only required in the finalize function, which clearly does not provide an...

Owner can steal input tokens

Lines of code Vulnerability details Impact Owner is in full control over the saleRecipient address. When a buy transaction enters the mempool, an owner can frontrun the buy with a transaction that calls setTokenOutPrice and sets the price to a very high value, effectively making bought tokens clo...

Owner never calls finalize() = rug pull

Lines of code Vulnerability details Impact In order for users to claim their promised tokenOut tokens, the contract owner must call the finalize function. If the owner never calls the finalize function, no user can call the claim function to get their tokens. The owner can call the sweep function...

[WP-H2] Funds can be frozen when critical key holders lose access to their keys

Lines of code Vulnerability details The current implementation requires trusted key holders Owner to send transactions finalize to finalize the sale before the buyers can claim the tokenOut from the contract. function finalize external onlyOwner require!finalized, "TokenSale: already finalized";...

Sale should be able to be automatically finalized

Lines of code Vulnerability details Impact The admins are the only ones that are able to finalize the sale. Throughout the token buying process, the admins are transferred tokenIn tokens, and the users are expecting to be able to get the corresponding tokenOut tokens once the sale is finalized. T...

TokenInLimit can be set arbitrarily high even if the contract doesn't have enough tokens to sell.

Lines of code Vulnerability details Impact If totalTokenOutBought tokenOut.balanceOfaddressthis the finalize function will forever revert and no-one will be able to claim their bought tokens. Proof of Concept TokenInLimit can be set arbitrarily high even if the contract doesn't have enough tokens...

Sale recipient can be changed mid-sale

Lines of code Vulnerability details Impact The setSaleRecipient function can change the sale recipient while a sale is open. This function can be only called by the owner of the contract, however, since: 1. the saleRecipient is the address that receives all the input tokens deposited by users, an...

No minOutAmount amount checks when buying

Lines of code Vulnerability details The tokenOutPrice can be changed with the setTokenOutPrice even if the sale is already running. Users might accept the current token price, send a purchase transaction, and before it is mined the token price can be un-intentionally changed. The user might recei...

Zero tokenOut balance = rug pull

Lines of code Vulnerability details Impact The only time that the Badger Citadel contract checks that the balance of tokenOut is greater than or equal to totalTokenOutBought is in the finalize function, which happens at the end of the sale. A contract owner can start a token sale but never send...

No guarantee sale organizer will fulfil their end of the deal

Lines of code Vulnerability details Impact Sale participants will only be able to claim their CTDL tokens once the sale is finalized. However, there is no guarantee that it ever will be, because: Sale finalisation can only be performed by the owner The owner is able to change the sale parameters...